From c777ba30677c7a66023dfeb1db01097809bd4757 Mon Sep 17 00:00:00 2001 From: Luke Kysow Date: Thu, 12 Oct 2017 21:36:09 -0700 Subject: [PATCH] Update to latest version of go-github (#157) --- Gopkg.lock | 2 +- server/server.go | 60 +++++---------- .../github.com/google/go-github/.travis.yml | 1 + .../google/go-github/example/appengine/app.go | 3 +- .../go-github/github/github-accessors.go | 32 ++++++++ .../google/go-github/github/github.go | 2 +- .../google/go-github/github/issues.go | 14 +++- .../google/go-github/github/issues_test.go | 11 +++ .../google/go-github/github/messages.go | 41 +++++++++- .../google/go-github/github/messages_test.go | 74 +++++++++++++++++++ .../google/go-github/github/orgs_members.go | 2 +- .../go-github/github/orgs_members_test.go | 4 +- .../google/go-github/github/repos.go | 8 +- .../google/go-github/github/timestamp_test.go | 10 ++- 14 files changed, 205 insertions(+), 59 deletions(-) diff --git a/Gopkg.lock b/Gopkg.lock index 9046c295f..a66add6c7 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -35,7 +35,7 @@ branch = "master" name = "github.com/google/go-github" packages = ["github"] - revision = "12126fbfe000a09047ebdeb2b5160be3af88ab9e" + revision = "c004bf0c02fcab9d87a7f2e2f002fd008bf0784c" [[projects]] branch = "master" diff --git a/server/server.go b/server/server.go index 202783219..d2b84d115 100644 --- a/server/server.go +++ b/server/server.go @@ -9,7 +9,6 @@ import ( "strings" "time" - "bytes" "io/ioutil" "github.com/elazarl/go-bindata-assetfs" @@ -23,7 +22,7 @@ import ( "github.com/hootsuite/atlantis/run" "github.com/hootsuite/atlantis/static" "github.com/hootsuite/atlantis/terraform" - homedir "github.com/mitchellh/go-homedir" + "github.com/mitchellh/go-homedir" "github.com/pkg/errors" "github.com/urfave/cli" "github.com/urfave/negroni" @@ -295,51 +294,32 @@ func (s *Server) postEvents(w http.ResponseWriter, r *http.Request) { githubReqID := "X-Github-Delivery=" + r.Header.Get("X-Github-Delivery") var payload []byte - // webhook requests can either be application/json or application/x-www-form-urlencoded. - // We accept both to make it easier on users that may choose x-www-form-urlencoded by mistake - // todo: use go-github's ValidatePayload method if https://github.com/google/go-github/pull/693 is merged - if r.Header.Get("Content-Type") == "application/x-www-form-urlencoded" { - // GitHub stores the json payload as a form value - payloadForm := r.FormValue("payload") - if payloadForm == "" { - s.respond(w, logging.Warn, http.StatusBadRequest, "request did not contain expected 'payload' form value") + // If we need to validate the Webhook secret, we can use go-github's + // ValidatePayload method. Otherwise we need to parse the request ourselves. + if len(s.githubWebHookSecret) != 0 { + var err error + if payload, err = gh.ValidatePayload(r, s.githubWebHookSecret); err != nil { + s.respond(w, logging.Warn, http.StatusBadRequest, "webhook request failed secret key validation") return } - if len(s.githubWebHookSecret) != 0 { - // github calculates the signature based on the query escaped - // post body. In order to use go-github's ValidatePayload method - // that only accepts an http request we need to override r.Body - // with a value that was the original raw body before it was - // parsed. - rawPayload := fmt.Sprintf("payload=%s", url.QueryEscape(payloadForm)) - r.Body = ioutil.NopCloser(bytes.NewBuffer([]byte(rawPayload))) - _, err := gh.ValidatePayload(r, s.githubWebHookSecret) - if err != nil { - s.respond(w, logging.Warn, http.StatusBadRequest, "webhook failed secret key validation") - return - } - } - payload = []byte(payloadForm) } else { - // else read it as json - if len(s.githubWebHookSecret) != 0 { + switch ct := r.Header.Get("Content-Type"); ct { + case "application/json": var err error - payload, err = gh.ValidatePayload(r, s.githubWebHookSecret) - if err != nil { - s.respond(w, logging.Warn, http.StatusBadRequest, "webhook failed secret key validation") - return - } - } else { - // if we're not validating against the webhook secret then - // we can't use the ValidatePayload method and need to read - // the request body ourselves. - defer r.Body.Close() - var err error - payload, err = ioutil.ReadAll(r.Body) - if err != nil { + if payload, err = ioutil.ReadAll(r.Body); err != nil { s.respond(w, logging.Warn, http.StatusBadRequest, "could not read body: %s", err) return } + case "application/x-www-form-urlencoded": + // GitHub stores the json payload as a form value + payloadForm := r.FormValue("payload") + if payloadForm == "" { + s.respond(w, logging.Warn, http.StatusBadRequest, "webhook request did not contain expected 'payload' form value") + return + } + payload = []byte(payloadForm) + default: + s.respond(w, logging.Warn, http.StatusBadRequest, fmt.Sprintf("webhook request has unsupported Content-Type %q", ct)) } } diff --git a/vendor/github.com/google/go-github/.travis.yml b/vendor/github.com/google/go-github/.travis.yml index 60ce6caef..f575d459e 100644 --- a/vendor/github.com/google/go-github/.travis.yml +++ b/vendor/github.com/google/go-github/.travis.yml @@ -1,6 +1,7 @@ sudo: false language: go go: + - 1.9.x - 1.8.x - 1.7.x - master diff --git a/vendor/github.com/google/go-github/example/appengine/app.go b/vendor/github.com/google/go-github/example/appengine/app.go index d14176d00..4d6cc30ce 100644 --- a/vendor/github.com/google/go-github/example/appengine/app.go +++ b/vendor/github.com/google/go-github/example/appengine/app.go @@ -3,7 +3,8 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -// The demo app shows how to use the github package on Google App Engine. +// Package demo provides an app that shows how to use the github package on +// Google App Engine. package demo import ( diff --git a/vendor/github.com/google/go-github/github/github-accessors.go b/vendor/github.com/google/go-github/github/github-accessors.go index 4fed8e50a..6da009baf 100644 --- a/vendor/github.com/google/go-github/github/github-accessors.go +++ b/vendor/github.com/google/go-github/github/github-accessors.go @@ -2100,6 +2100,14 @@ func (i *Issue) GetComments() int { return *i.Comments } +// GetCommentsURL returns the CommentsURL field if it's non-nil, zero value otherwise. +func (i *Issue) GetCommentsURL() string { + if i == nil || i.CommentsURL == nil { + return "" + } + return *i.CommentsURL +} + // GetCreatedAt returns the CreatedAt field if it's non-nil, zero value otherwise. func (i *Issue) GetCreatedAt() time.Time { if i == nil || i.CreatedAt == nil { @@ -2108,6 +2116,14 @@ func (i *Issue) GetCreatedAt() time.Time { return *i.CreatedAt } +// GetEventsURL returns the EventsURL field if it's non-nil, zero value otherwise. +func (i *Issue) GetEventsURL() string { + if i == nil || i.EventsURL == nil { + return "" + } + return *i.EventsURL +} + // GetHTMLURL returns the HTMLURL field if it's non-nil, zero value otherwise. func (i *Issue) GetHTMLURL() string { if i == nil || i.HTMLURL == nil { @@ -2124,6 +2140,14 @@ func (i *Issue) GetID() int { return *i.ID } +// GetLabelsURL returns the LabelsURL field if it's non-nil, zero value otherwise. +func (i *Issue) GetLabelsURL() string { + if i == nil || i.LabelsURL == nil { + return "" + } + return *i.LabelsURL +} + // GetLocked returns the Locked field if it's non-nil, zero value otherwise. func (i *Issue) GetLocked() bool { if i == nil || i.Locked == nil { @@ -2140,6 +2164,14 @@ func (i *Issue) GetNumber() int { return *i.Number } +// GetRepositoryURL returns the RepositoryURL field if it's non-nil, zero value otherwise. +func (i *Issue) GetRepositoryURL() string { + if i == nil || i.RepositoryURL == nil { + return "" + } + return *i.RepositoryURL +} + // GetState returns the State field if it's non-nil, zero value otherwise. func (i *Issue) GetState() string { if i == nil || i.State == nil { diff --git a/vendor/github.com/google/go-github/github/github.go b/vendor/github.com/google/go-github/github/github.go index 99ad8d078..ae37ba8f6 100644 --- a/vendor/github.com/google/go-github/github/github.go +++ b/vendor/github.com/google/go-github/github/github.go @@ -27,7 +27,7 @@ import ( ) const ( - libraryVersion = "11" + libraryVersion = "12" defaultBaseURL = "https://api.github.com/" uploadBaseURL = "https://uploads.github.com/" userAgent = "go-github/" + libraryVersion diff --git a/vendor/github.com/google/go-github/github/issues.go b/vendor/github.com/google/go-github/github/issues.go index b437d5063..9b7a9d687 100644 --- a/vendor/github.com/google/go-github/github/issues.go +++ b/vendor/github.com/google/go-github/github/issues.go @@ -23,6 +23,7 @@ type IssuesService service // but not every issue is a pull request. Some endpoints, events, and webhooks // may also return pull requests via this struct. If PullRequestLinks is nil, // this is an issue, and if PullRequestLinks is not nil, this is a pull request. +// The IsPullRequest helper method can be used to check that. type Issue struct { ID *int `json:"id,omitempty"` Number *int `json:"number,omitempty"` @@ -40,6 +41,10 @@ type Issue struct { ClosedBy *User `json:"closed_by,omitempty"` URL *string `json:"url,omitempty"` HTMLURL *string `json:"html_url,omitempty"` + CommentsURL *string `json:"comments_url,omitempty"` + EventsURL *string `json:"events_url,omitempty"` + LabelsURL *string `json:"labels_url,omitempty"` + RepositoryURL *string `json:"repository_url,omitempty"` Milestone *Milestone `json:"milestone,omitempty"` PullRequestLinks *PullRequestLinks `json:"pull_request,omitempty"` Repository *Repository `json:"repository,omitempty"` @@ -55,6 +60,13 @@ func (i Issue) String() string { return Stringify(i) } +// IsPullRequest reports whether the issue is also a pull request. It uses the +// method recommended by GitHub's API documentation, which is to check whether +// PullRequestLinks is non-nil. +func (i Issue) IsPullRequest() bool { + return i.PullRequestLinks != nil +} + // IssueRequest represents a request to create/edit an issue. // It is separate from Issue above because otherwise Labels // and Assignee fail to serialize to the correct JSON. @@ -97,7 +109,7 @@ type IssueListOptions struct { } // PullRequestLinks object is added to the Issue object when it's an issue included -// in the IssueCommentEvent webhook payload, if the webhooks is fired by a comment on a PR +// in the IssueCommentEvent webhook payload, if the webhook is fired by a comment on a PR. type PullRequestLinks struct { URL *string `json:"url,omitempty"` HTMLURL *string `json:"html_url,omitempty"` diff --git a/vendor/github.com/google/go-github/github/issues_test.go b/vendor/github.com/google/go-github/github/issues_test.go index dadee7557..da60740fb 100644 --- a/vendor/github.com/google/go-github/github/issues_test.go +++ b/vendor/github.com/google/go-github/github/issues_test.go @@ -275,3 +275,14 @@ func TestIssuesService_Unlock(t *testing.T) { t.Errorf("Issues.Unlock returned error: %v", err) } } + +func TestIsPullRequest(t *testing.T) { + i := new(Issue) + if i.IsPullRequest() == true { + t.Errorf("expected i.IsPullRequest (%v) to return false, got true", i) + } + i.PullRequestLinks = &PullRequestLinks{URL: String("http://example.com")} + if i.IsPullRequest() == false { + t.Errorf("expected i.IsPullRequest (%v) to return true, got false", i) + } +} diff --git a/vendor/github.com/google/go-github/github/messages.go b/vendor/github.com/google/go-github/github/messages.go index c0f315a77..3a4d46440 100644 --- a/vendor/github.com/google/go-github/github/messages.go +++ b/vendor/github.com/google/go-github/github/messages.go @@ -20,6 +20,7 @@ import ( "hash" "io/ioutil" "net/http" + "net/url" "strings" ) @@ -122,6 +123,8 @@ func messageMAC(signature string) ([]byte, func() hash.Hash, error) { // ValidatePayload validates an incoming GitHub Webhook event request // and returns the (JSON) payload. +// The Content-Type header of the payload can be "application/json" or "application/x-www-form-urlencoded". +// If the Content-Type is neither then an error is returned. // secretKey is the GitHub Webhook secret message. // // Example usage: @@ -133,13 +136,43 @@ func messageMAC(signature string) ([]byte, func() hash.Hash, error) { // } // func ValidatePayload(r *http.Request, secretKey []byte) (payload []byte, err error) { - payload, err = ioutil.ReadAll(r.Body) - if err != nil { - return nil, err + var body []byte // Raw body that GitHub uses to calculate the signature. + + switch ct := r.Header.Get("Content-Type"); ct { + case "application/json": + var err error + if body, err = ioutil.ReadAll(r.Body); err != nil { + return nil, err + } + + // If the content type is application/json, + // the JSON payload is just the original body. + payload = body + + case "application/x-www-form-urlencoded": + // payloadFormParam is the name of the form parameter that the JSON payload + // will be in if a webhook has its content type set to application/x-www-form-urlencoded. + const payloadFormParam = "payload" + + var err error + if body, err = ioutil.ReadAll(r.Body); err != nil { + return nil, err + } + + // If the content type is application/x-www-form-urlencoded, + // the JSON payload will be under the "payload" form param. + form, err := url.ParseQuery(string(body)) + if err != nil { + return nil, err + } + payload = []byte(form.Get(payloadFormParam)) + + default: + return nil, fmt.Errorf("Webhook request has unsupported Content-Type %q", ct) } sig := r.Header.Get(signatureHeader) - if err := validateSignature(sig, payload, secretKey); err != nil { + if err := validateSignature(sig, body, secretKey); err != nil { return nil, err } return payload, nil diff --git a/vendor/github.com/google/go-github/github/messages_test.go b/vendor/github.com/google/go-github/github/messages_test.go index 5f348f2aa..eebfa10d3 100644 --- a/vendor/github.com/google/go-github/github/messages_test.go +++ b/vendor/github.com/google/go-github/github/messages_test.go @@ -9,7 +9,9 @@ import ( "bytes" "encoding/json" "net/http" + "net/url" "reflect" + "strings" "testing" ) @@ -68,6 +70,7 @@ func TestValidatePayload(t *testing.T) { if test.signature != "" { req.Header.Set(signatureHeader, test.signature) } + req.Header.Set("Content-Type", "application/json") got, err := ValidatePayload(req, secretKey) if err != nil { @@ -82,6 +85,77 @@ func TestValidatePayload(t *testing.T) { } } +func TestValidatePayload_FormGet(t *testing.T) { + payload := `{"yo":true}` + signature := "sha1=3374ef144403e8035423b23b02e2c9d7a4c50368" + secretKey := []byte("0123456789abcdef") + + form := url.Values{} + form.Add("payload", payload) + req, err := http.NewRequest("POST", "http://localhost/event", strings.NewReader(form.Encode())) + if err != nil { + t.Fatalf("NewRequest: %v", err) + } + req.PostForm = form + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + req.Header.Set(signatureHeader, signature) + + got, err := ValidatePayload(req, secretKey) + if err != nil { + t.Errorf("ValidatePayload(%#v): err = %v, want nil", payload, err) + } + if string(got) != payload { + t.Errorf("ValidatePayload = %q, want %q", got, payload) + } + + // check that if payload is invalid we get error + req.Header.Set(signatureHeader, "invalid signature") + if _, err = ValidatePayload(req, nil); err == nil { + t.Error("ValidatePayload = nil, want err") + } +} + +func TestValidatePayload_FormPost(t *testing.T) { + payload := `{"yo":true}` + signature := "sha1=3374ef144403e8035423b23b02e2c9d7a4c50368" + secretKey := []byte("0123456789abcdef") + + form := url.Values{} + form.Set("payload", payload) + buf := bytes.NewBufferString(form.Encode()) + req, err := http.NewRequest("POST", "http://localhost/event", buf) + if err != nil { + t.Fatalf("NewRequest: %v", err) + } + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + req.Header.Set(signatureHeader, signature) + + got, err := ValidatePayload(req, secretKey) + if err != nil { + t.Errorf("ValidatePayload(%#v): err = %v, want nil", payload, err) + } + if string(got) != payload { + t.Errorf("ValidatePayload = %q, want %q", got, payload) + } + + // check that if payload is invalid we get error + req.Header.Set(signatureHeader, "invalid signature") + if _, err = ValidatePayload(req, nil); err == nil { + t.Error("ValidatePayload = nil, want err") + } +} + +func TestValidatePayload_InvalidContentType(t *testing.T) { + req, err := http.NewRequest("POST", "http://localhost/event", nil) + if err != nil { + t.Fatalf("NewRequest: %v", err) + } + req.Header.Set("Content-Type", "invalid content type") + if _, err = ValidatePayload(req, nil); err == nil { + t.Error("ValidatePayload = nil, want err") + } +} + func TestParseWebHook(t *testing.T) { tests := []struct { payload interface{} diff --git a/vendor/github.com/google/go-github/github/orgs_members.go b/vendor/github.com/google/go-github/github/orgs_members.go index f1209c7c4..d0ea6a985 100644 --- a/vendor/github.com/google/go-github/github/orgs_members.go +++ b/vendor/github.com/google/go-github/github/orgs_members.go @@ -278,7 +278,7 @@ func (s *OrganizationsService) RemoveOrgMembership(ctx context.Context, user, or // ListPendingOrgInvitations returns a list of pending invitations. // // GitHub API docs: https://developer.github.com/v3/orgs/members/#list-pending-organization-invitations -func (s *OrganizationsService) ListPendingOrgInvitations(ctx context.Context, org int, opt *ListOptions) ([]*Invitation, *Response, error) { +func (s *OrganizationsService) ListPendingOrgInvitations(ctx context.Context, org string, opt *ListOptions) ([]*Invitation, *Response, error) { u := fmt.Sprintf("orgs/%v/invitations", org) u, err := addOptions(u, opt) if err != nil { diff --git a/vendor/github.com/google/go-github/github/orgs_members_test.go b/vendor/github.com/google/go-github/github/orgs_members_test.go index a17c51865..21fbeeaf3 100644 --- a/vendor/github.com/google/go-github/github/orgs_members_test.go +++ b/vendor/github.com/google/go-github/github/orgs_members_test.go @@ -360,7 +360,7 @@ func TestOrganizationsService_ListPendingOrgInvitations(t *testing.T) { setup() defer teardown() - mux.HandleFunc("/orgs/1/invitations", func(w http.ResponseWriter, r *http.Request) { + mux.HandleFunc("/orgs/o/invitations", func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, "GET") testFormValues(t, r, values{"page": "1"}) fmt.Fprint(w, `[ @@ -394,7 +394,7 @@ func TestOrganizationsService_ListPendingOrgInvitations(t *testing.T) { }) opt := &ListOptions{Page: 1} - invitations, _, err := client.Organizations.ListPendingOrgInvitations(context.Background(), 1, opt) + invitations, _, err := client.Organizations.ListPendingOrgInvitations(context.Background(), "o", opt) if err != nil { t.Errorf("Organizations.ListPendingOrgInvitations returned error: %v", err) } diff --git a/vendor/github.com/google/go-github/github/repos.go b/vendor/github.com/google/go-github/github/repos.go index 3846335b3..b6d98cc16 100644 --- a/vendor/github.com/google/go-github/github/repos.go +++ b/vendor/github.com/google/go-github/github/repos.go @@ -558,7 +558,7 @@ type RequiredStatusChecks struct { // PullRequestReviewsEnforcement represents the pull request reviews enforcement of a protected branch. type PullRequestReviewsEnforcement struct { - // Specifies which users and teams can dismiss pull requets reviews. + // Specifies which users and teams can dismiss pull request reviews. DismissalRestrictions DismissalRestrictions `json:"dismissal_restrictions"` // Specifies if approved reviews are dismissed automatically, when a new commit is pushed. DismissStaleReviews bool `json:"dismiss_stale_reviews"` @@ -568,7 +568,7 @@ type PullRequestReviewsEnforcement struct { // enforcement of a protected branch. It is separate from PullRequestReviewsEnforcement above // because the request structure is different from the response structure. type PullRequestReviewsEnforcementRequest struct { - // Specifies which users and teams should be allowed to dismiss pull requets reviews. Can be nil to disable the restrictions. + // Specifies which users and teams should be allowed to dismiss pull request reviews. Can be nil to disable the restrictions. DismissalRestrictionsRequest *DismissalRestrictionsRequest `json:"dismissal_restrictions"` // Specifies if approved reviews can be dismissed automatically, when a new commit is pushed. (Required) DismissStaleReviews bool `json:"dismiss_stale_reviews"` @@ -601,9 +601,9 @@ func (req PullRequestReviewsEnforcementRequest) MarshalJSON() ([]byte, error) { // enforcement of a protected branch. It is separate from PullRequestReviewsEnforcementRequest above // because the patch request does not require all fields to be initialized. type PullRequestReviewsEnforcementUpdate struct { - // Specifies which users and teams can dismiss pull requets reviews. Can be ommitted. + // Specifies which users and teams can dismiss pull request reviews. Can be omitted. DismissalRestrictionsRequest *DismissalRestrictionsRequest `json:"dismissal_restrictions,omitempty"` - // Specifies if approved reviews can be dismissed automatically, when a new commit is pushed. Can be ommited. + // Specifies if approved reviews can be dismissed automatically, when a new commit is pushed. Can be omitted. DismissStaleReviews *bool `json:"dismiss_stale_reviews,omitempty"` } diff --git a/vendor/github.com/google/go-github/github/timestamp_test.go b/vendor/github.com/google/go-github/github/timestamp_test.go index 997b43db8..ea7fa0eb5 100644 --- a/vendor/github.com/google/go-github/github/timestamp_test.go +++ b/vendor/github.com/google/go-github/github/timestamp_test.go @@ -13,9 +13,10 @@ import ( ) const ( - emptyTimeStr = `"0001-01-01T00:00:00Z"` - referenceTimeStr = `"2006-01-02T15:04:05Z"` - referenceUnixTimeStr = `1136214245` + emptyTimeStr = `"0001-01-01T00:00:00Z"` + referenceTimeStr = `"2006-01-02T15:04:05Z"` + referenceTimeStrFractional = `"2006-01-02T15:04:05.000Z"` // This format was returned by the Projects API before October 1, 2017. + referenceUnixTimeStr = `1136214245` ) var ( @@ -57,7 +58,8 @@ func TestTimestamp_Unmarshal(t *testing.T) { equal bool }{ {"Reference", referenceTimeStr, Timestamp{referenceTime}, false, true}, - {"ReferenceUnix", `1136214245`, Timestamp{referenceTime}, false, true}, + {"ReferenceUnix", referenceUnixTimeStr, Timestamp{referenceTime}, false, true}, + {"ReferenceFractional", referenceTimeStrFractional, Timestamp{referenceTime}, false, true}, {"Empty", emptyTimeStr, Timestamp{}, false, true}, {"UnixStart", `0`, Timestamp{unixOrigin}, false, true}, {"Mismatch", referenceTimeStr, Timestamp{}, false, false},