-
Notifications
You must be signed in to change notification settings - Fork 18
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
Chore: add sendgrid webhook endpoints to receive webhook events #1150
Chore: add sendgrid webhook endpoints to receive webhook events #1150
Conversation
return types.NewErrHttp(http.StatusMethodNotAllowed, "Invalid request method") | ||
} | ||
|
||
inboundEmail, err := inbound.Parse(req.Request) |
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.
Sendgrid doesn't have a way to attach secret header so it is a bit tricky to vefify whether requests are coming from sendgrid...this makes our webhook server vulnerable to unverified webhook events from other places. (at least not from our account that you can configure to send a secret key as a header)
so there is no validation right to validate if requests are actually coming from sendgrid.
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.
I added basic auth protection for now. The username and password can be configured at global level to protect webhook endpoint, and sendgrid webhook can be configured to have configured username and password in front of the url.
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.
Does twilio's event webhook signing feature not work for this? IIUC, enabling this will add a X-Twilio-Email-Event-Webhook-Signature
header containing a verification signature to all webhook requests it makes.
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.
yeah, that's for their generic event webhook. The inbound parse webhook doesn't support it.
@@ -0,0 +1,132 @@ | |||
package emailtrigger |
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.
I refactored the code a bit to be able to share some common trigger code from both smtp and sendgrid.
83c04f0
to
4b99147
Compare
Signed-off-by: Daishan Peng <[email protected]>
4b99147
to
f58a331
Compare
Signed-off-by: Daishan Peng <[email protected]>
pkg/api/handlers/sendgrid/webhook.go
Outdated
if req.Request.Method != http.MethodPost { | ||
return types.NewErrHttp(http.StatusMethodNotAllowed, "Invalid request method") | ||
} |
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.
nit: this is handled when we set up the route.
pkg/api/router/router.go
Outdated
@@ -256,6 +259,9 @@ func Router(services *services.Services) (http.Handler, error) { | |||
mux.HandleFunc("POST /api/webhooks/{id}/remove-token", webhooks.RemoveToken) | |||
mux.HandleFunc("POST /api/webhooks/{namespace}/{id}", webhooks.Execute) | |||
|
|||
// Webhook for third party integration to trigger workflow | |||
mux.HandleFunc("POST /webhook/sendgrid", sendgridWebhookHandler.InboundWebhookHandler) |
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.
We have the convention that all of our backend APIs start with /api
. Can this one be /api/sendgrid
?
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.
yeah, I am good with that.
pkg/services/config.go
Outdated
SendgridWebhookUsername string `usage:"The username for the sendgrid webhook to authenticate with" env:"OBOT_SENDGRID_WEBHOOK_USERNAME"` | ||
SendgridWebhookPassword string `usage:"The password for the sendgrid webhook to authenticate with" env:"OBOT_SENDGRID_WEBHOOK_PASSWORD"` |
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.
All the other configuration variables we use to configure the server start with OBOT_SERVER_
. If you remove the env
tag here, that will be the same for these configuration variables.
Signed-off-by: Daishan Peng <[email protected]>
Add sendgrid webhook endpoint to be able to receive sendgrid inbound parse event and trigger workflow.
A use case would be getting emails regarding to receipts and have dedicated workflows processing these emails.