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

Adding github webhook secret key option. Fixes #34 #119

Closed
wants to merge 2 commits into from

Conversation

anubhavmishra
Copy link
Collaborator

@anubhavmishra anubhavmishra commented Aug 13, 2017

  • Adding new CLI flag gh-webhook-secret-key to supply Github webhook secret key to verify payloads.

TODO:

  • Add a docs about the flag in README.md

@@ -301,6 +303,15 @@ func (s *Server) postEvents(w http.ResponseWriter, r *http.Request) {
githubReqID := "X-Github-Delivery=" + r.Header.Get("X-Github-Delivery")
var payload []byte

// validate payload if github webhook secret key is set
if s.eventParser.GithubWehbookSecretKey != "" {
_, err := gh.ValidatePayload(r, []byte(s.eventParser.GithubWehbookSecretKey))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this doesn't work if the webhook is set up using x-www-form-urlencoded

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would right? I am checking for secret key before we even determine whether the request is Content-Type is json or x-www-form-urlencoded.

@lkysow
Copy link
Collaborator

lkysow commented Aug 14, 2017

Can we close this in favor of #120?

@anubhavmishra
Copy link
Collaborator Author

Yes sir. 👍

@lkysow
Copy link
Collaborator

lkysow commented Aug 14, 2017 via email

@lkysow lkysow deleted the enable-webhook-secret branch August 14, 2017 16:31
malnick pushed a commit to BlueOwlDev/atlantis that referenced this pull request Jun 21, 2018
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