From c1b6c7aac7904c32efb868d898234a2cc735059b Mon Sep 17 00:00:00 2001 From: Stefan Mees Date: Fri, 12 Jun 2020 17:03:03 +0200 Subject: [PATCH 1/2] skip unnecessary json parsing if ignoredUser and allowedUser properties are empty --- pkg/proxy/proxy.go | 16 +++++++++------- pkg/proxy/proxy_test.go | 14 ++++++++++++++ 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/pkg/proxy/proxy.go b/pkg/proxy/proxy.go index 855b3c8..fa72e51 100644 --- a/pkg/proxy/proxy.go +++ b/pkg/proxy/proxy.go @@ -142,13 +142,15 @@ func (p *Proxy) proxyRequest(w http.ResponseWriter, r *http.Request, params http return } - committer := provider.GetCommitter(*hook) - log.Printf("Incoming request from user: %s", committer) - if p.isIgnoredUser(committer) || (!p.isAllowedUser(committer)) { - log.Printf("Ignoring request for user: %s", committer) - w.WriteHeader(http.StatusOK) - w.Write([]byte(fmt.Sprintf("Ignoring request for user: %s", committer))) - return + if len(p.ignoredUsers) > 0 || len(p.allowedUsers) > 0 { + committer := provider.GetCommitter(*hook) + log.Printf("Incoming request from user: %s", committer) + if p.isIgnoredUser(committer) || (!p.isAllowedUser(committer)) { + log.Printf("Ignoring request for user: %s", committer) + w.WriteHeader(http.StatusOK) + w.Write([]byte(fmt.Sprintf("Ignoring request for user: %s", committer))) + return + } } if len(strings.TrimSpace(p.secret)) > 0 && !provider.Validate(*hook) { diff --git a/pkg/proxy/proxy_test.go b/pkg/proxy/proxy_test.go index 787dc6a..4e9ceb0 100644 --- a/pkg/proxy/proxy_test.go +++ b/pkg/proxy/proxy_test.go @@ -603,6 +603,20 @@ func TestProxy_proxyRequest(t *testing.T) { }, wantStatusCode: http.StatusMethodNotAllowed, }, + { + name: "TestProxyRequestShouldNotParseJsonWithoutAllowedOrIgnoredUsersConfigured", + fields: fields{ + provider: providers.GitlabProviderKind, + upstreamURL: httpBinURLSecure, + allowedPaths: []string{}, + secret: "", + }, + args: args{ + request: createGitlabRequestWithPayload(http.MethodPost, "/post", + proxyGitlabTestSecret, proxyGitlabTestEvent, []byte("{}")), + }, + wantStatusCode: http.StatusOK, + }, { name: "TestProxyRequestWithInvalidHttpMethod", fields: fields{ From db828bcb53424eb33b71bb8599850bc210e4619f Mon Sep 17 00:00:00 2001 From: Stefan Mees Date: Mon, 15 Jun 2020 16:08:36 +0200 Subject: [PATCH 2/2] skip unnecessary json parsing if ignoredUser and allowedUser properties are empty --- pkg/providers/gitlab.go | 2 +- pkg/proxy/proxy_test.go | 19 ++++++++++++++++++- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/pkg/providers/gitlab.go b/pkg/providers/gitlab.go index 8adb05a..a95b206 100644 --- a/pkg/providers/gitlab.go +++ b/pkg/providers/gitlab.go @@ -63,7 +63,7 @@ func (p *GitlabProvider) Validate(hook Hook) bool { func (p *GitlabProvider) GetCommitter(hook Hook) string { var payloadData GitlabPushPayload if err := json.Unmarshal(hook.Payload, &payloadData); err != nil { - log.Printf("Gitlab hook payload unmarshalling failed") + log.Printf("Gitlab hook payload unmarshalling failed: %v", err) return "" } diff --git a/pkg/proxy/proxy_test.go b/pkg/proxy/proxy_test.go index 4e9ceb0..c12fe3f 100644 --- a/pkg/proxy/proxy_test.go +++ b/pkg/proxy/proxy_test.go @@ -15,7 +15,7 @@ import ( const ( proxyGitlabTestSecret = "testSecret" - proxyGitlabTestEvent = "testEvent" + proxyGitlabTestEvent = "Push Hook" proxyGitlabTestBody = "testBody" httpBinURL = "httpbin.org" httpBinURLInsecure = "http://" + httpBinURL @@ -468,6 +468,7 @@ func TestProxy_proxyRequest(t *testing.T) { upstreamURL string allowedPaths []string secret string + allowedUsers []string } type args struct { request *http.Request @@ -617,6 +618,21 @@ func TestProxy_proxyRequest(t *testing.T) { }, wantStatusCode: http.StatusOK, }, + { + name: "TestProxyRequestShouldParseJsonWithAllowedOrIgnoredUsersConfigured", + fields: fields{ + provider: providers.GitlabProviderKind, + upstreamURL: httpBinURLSecure, + allowedPaths: []string{}, + secret: "", + allowedUsers: []string{"jsmith"}, + }, + args: args{ + request: createGitlabRequestWithPayload(http.MethodPost, "/post", + proxyGitlabTestSecret, proxyGitlabTestEvent, proxyGitlabTestPayload), + }, + wantStatusCode: http.StatusOK, + }, { name: "TestProxyRequestWithInvalidHttpMethod", fields: fields{ @@ -751,6 +767,7 @@ func TestProxy_proxyRequest(t *testing.T) { upstreamURL: tt.fields.upstreamURL, allowedPaths: tt.fields.allowedPaths, secret: tt.fields.secret, + allowedUsers: tt.fields.allowedUsers, } router := httprouter.New() router.POST("/*path", p.proxyRequest)