From 411adf6f21ac6de81a2ade696b53b8d4d012f6a4 Mon Sep 17 00:00:00 2001 From: Henry Jenkins Date: Sun, 23 Jun 2019 20:40:59 +0100 Subject: [PATCH 1/4] Switch linter to golangci-lint --- .golangci.yml | 14 ++++++++++++++ .travis.yml | 3 +-- CHANGELOG.md | 1 + Dockerfile | 1 + Dockerfile.arm64 | 1 + Dockerfile.armv6 | 1 + Makefile | 12 +----------- configure | 4 ++-- 8 files changed, 22 insertions(+), 15 deletions(-) create mode 100644 .golangci.yml diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000..4dc2d94 --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,14 @@ +run: + deadline: 120s +linters: + enable: + - govet + # TODO: Not supported by golang-ci - vetshadow + - golint + - ineffassign + - goconst + - deadcode + - gofmt + - goimports + enable-all: false + disable-all: true diff --git a/.travis.yml b/.travis.yml index d1151db..445eb11 100644 --- a/.travis.yml +++ b/.travis.yml @@ -6,8 +6,7 @@ install: - wget -O dep https://github.com/golang/dep/releases/download/v0.5.0/dep-linux-amd64 - chmod +x dep - mv dep $GOPATH/bin/dep - - go get github.com/alecthomas/gometalinter - - gometalinter --install + - curl -sfL https://install.goreleaser.com/github.com/golangci/golangci-lint.sh | sh -s -- -b $GOPATH/bin v1.17.1 script: - ./configure && make test sudo: false diff --git a/CHANGELOG.md b/CHANGELOG.md index 6858187..69d071f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -62,6 +62,7 @@ - [#185](https://github.com/pusher/oauth2_proxy/pull/185) Fix an unsupported protocol scheme error during token validation when using the Azure provider (@jonas) - [#141](https://github.com/pusher/oauth2_proxy/pull/141) Check google group membership based on email address (@bchess) - Google Group membership is additionally checked via email address, allowing users outside a GSuite domain to be authorized. +- [#XX](https://github.com/pusher/outh2_proxy/pull/XX) Switch from gometalinter to golangci-lint # v3.2.0 diff --git a/Dockerfile b/Dockerfile index f6649a9..757a1f5 100644 --- a/Dockerfile +++ b/Dockerfile @@ -3,6 +3,7 @@ FROM golang:1.12-stretch AS builder # Download tools RUN wget -O $GOPATH/bin/dep https://github.com/golang/dep/releases/download/v0.5.0/dep-linux-amd64 RUN chmod +x $GOPATH/bin/dep +RUN curl -sfL https://install.goreleaser.com/github.com/golangci/golangci-lint.sh | sh -s -- -b $(go env GOPATH)/bin v1.17.1 # Copy sources WORKDIR $GOPATH/src/github.com/pusher/oauth2_proxy diff --git a/Dockerfile.arm64 b/Dockerfile.arm64 index 7f011ba..f4bf495 100644 --- a/Dockerfile.arm64 +++ b/Dockerfile.arm64 @@ -3,6 +3,7 @@ FROM golang:1.12-stretch AS builder # Download tools RUN wget -O $GOPATH/bin/dep https://github.com/golang/dep/releases/download/v0.5.0/dep-linux-amd64 RUN chmod +x $GOPATH/bin/dep +RUN curl -sfL https://install.goreleaser.com/github.com/golangci/golangci-lint.sh | sh -s -- -b $(go env GOPATH)/bin v1.17.1 # Copy sources WORKDIR $GOPATH/src/github.com/pusher/oauth2_proxy diff --git a/Dockerfile.armv6 b/Dockerfile.armv6 index e2c57b9..32bb124 100644 --- a/Dockerfile.armv6 +++ b/Dockerfile.armv6 @@ -3,6 +3,7 @@ FROM golang:1.12-stretch AS builder # Download tools RUN wget -O $GOPATH/bin/dep https://github.com/golang/dep/releases/download/v0.5.0/dep-linux-amd64 RUN chmod +x $GOPATH/bin/dep +RUN curl -sfL https://install.goreleaser.com/github.com/golangci/golangci-lint.sh | sh -s -- -b $(go env GOPATH)/bin v1.17.1 # Copy sources WORKDIR $GOPATH/src/github.com/pusher/oauth2_proxy diff --git a/Makefile b/Makefile index 720fd4d..3286bda 100644 --- a/Makefile +++ b/Makefile @@ -17,17 +17,7 @@ distclean: clean .PHONY: lint lint: - $(GOMETALINTER) --vendor --disable-all \ - --enable=vet \ - --enable=vetshadow \ - --enable=golint \ - --enable=ineffassign \ - --enable=goconst \ - --enable=deadcode \ - --enable=gofmt \ - --enable=goimports \ - --deadline=120s \ - --tests ./... + $(GOLANGCILINT) run .PHONY: dep dep: diff --git a/configure b/configure index 5d74683..f1b8ae6 100755 --- a/configure +++ b/configure @@ -126,7 +126,7 @@ check_for go check_go_version check_go_env check_for dep -check_for gometalinter +check_for golangci-lint echo @@ -135,7 +135,7 @@ cat <<- EOF > .env GO := "${tools[go]}" GO_VERSION := ${tools[go_version]} DEP := "${tools[dep]}" - GOMETALINTER := "${tools[gometalinter]}" + GOLANGCILINT := "${tools[golangci-lint]}" EOF echo "Environment configuration written to .env" From d24aacdb5c34630a7a8ca3b2f9fe0ef1c093a37d Mon Sep 17 00:00:00 2001 From: Henry Jenkins Date: Sun, 23 Jun 2019 20:41:23 +0100 Subject: [PATCH 2/4] Fix lint errors --- http_test.go | 27 ++++++++++++++----------- oauthproxy.go | 17 +++++++++++++--- oauthproxy_test.go | 50 +++++++++++++++++++++++----------------------- options.go | 2 +- 4 files changed, 55 insertions(+), 41 deletions(-) diff --git a/http_test.go b/http_test.go index b365d6f..f5ee142 100644 --- a/http_test.go +++ b/http_test.go @@ -8,6 +8,9 @@ import ( "github.com/stretchr/testify/assert" ) +const localhost = "127.0.0.1" +const host = "test-server" + func TestGCPHealthcheckLiveness(t *testing.T) { handler := func(w http.ResponseWriter, req *http.Request) { w.Write([]byte("test")) @@ -16,8 +19,8 @@ func TestGCPHealthcheckLiveness(t *testing.T) { h := gcpHealthcheck(http.HandlerFunc(handler)) rw := httptest.NewRecorder() r, _ := http.NewRequest("GET", "/liveness_check", nil) - r.RemoteAddr = "127.0.0.1" - r.Host = "test-server" + r.RemoteAddr = localhost + r.Host = host h.ServeHTTP(rw, r) assert.Equal(t, 200, rw.Code) @@ -32,8 +35,8 @@ func TestGCPHealthcheckReadiness(t *testing.T) { h := gcpHealthcheck(http.HandlerFunc(handler)) rw := httptest.NewRecorder() r, _ := http.NewRequest("GET", "/readiness_check", nil) - r.RemoteAddr = "127.0.0.1" - r.Host = "test-server" + r.RemoteAddr = localhost + r.Host = host h.ServeHTTP(rw, r) assert.Equal(t, 200, rw.Code) @@ -48,8 +51,8 @@ func TestGCPHealthcheckNotHealthcheck(t *testing.T) { h := gcpHealthcheck(http.HandlerFunc(handler)) rw := httptest.NewRecorder() r, _ := http.NewRequest("GET", "/not_any_check", nil) - r.RemoteAddr = "127.0.0.1" - r.Host = "test-server" + r.RemoteAddr = localhost + r.Host = host h.ServeHTTP(rw, r) assert.Equal(t, "test", rw.Body.String()) @@ -63,8 +66,8 @@ func TestGCPHealthcheckIngress(t *testing.T) { h := gcpHealthcheck(http.HandlerFunc(handler)) rw := httptest.NewRecorder() r, _ := http.NewRequest("GET", "/", nil) - r.RemoteAddr = "127.0.0.1" - r.Host = "test-server" + r.RemoteAddr = localhost + r.Host = host r.Header.Set(userAgentHeader, googleHealthCheckUserAgent) h.ServeHTTP(rw, r) @@ -80,8 +83,8 @@ func TestGCPHealthcheckNotIngress(t *testing.T) { h := gcpHealthcheck(http.HandlerFunc(handler)) rw := httptest.NewRecorder() r, _ := http.NewRequest("GET", "/foo", nil) - r.RemoteAddr = "127.0.0.1" - r.Host = "test-server" + r.RemoteAddr = localhost + r.Host = host r.Header.Set(userAgentHeader, googleHealthCheckUserAgent) h.ServeHTTP(rw, r) @@ -96,8 +99,8 @@ func TestGCPHealthcheckNotIngressPut(t *testing.T) { h := gcpHealthcheck(http.HandlerFunc(handler)) rw := httptest.NewRecorder() r, _ := http.NewRequest("PUT", "/", nil) - r.RemoteAddr = "127.0.0.1" - r.Host = "test-server" + r.RemoteAddr = localhost + r.Host = host r.Header.Set(userAgentHeader, googleHealthCheckUserAgent) h.ServeHTTP(rw, r) diff --git a/oauthproxy.go b/oauthproxy.go index 99dfb36..da56cfc 100644 --- a/oauthproxy.go +++ b/oauthproxy.go @@ -160,7 +160,7 @@ func NewFileServer(path string, filesystemPath string) (proxy http.Handler) { } // NewWebSocketOrRestReverseProxy creates a reverse proxy for REST or websocket based on url -func NewWebSocketOrRestReverseProxy(u *url.URL, opts *Options, auth hmacauth.HmacAuth) (restProxy http.Handler) { +func NewWebSocketOrRestReverseProxy(u *url.URL, opts *Options, auth hmacauth.HmacAuth) http.Handler { u.Path = "" proxy := NewReverseProxy(u, opts.FlushInterval) if !opts.PassHostHeader { @@ -176,7 +176,12 @@ func NewWebSocketOrRestReverseProxy(u *url.URL, opts *Options, auth hmacauth.Hma wsURL := &url.URL{Scheme: wsScheme, Host: u.Host} wsProxy = wsutil.NewSingleHostReverseProxy(wsURL) } - return &UpstreamProxy{u.Host, proxy, wsProxy, auth} + return &UpstreamProxy{ + upstream: u.Host, + handler: proxy, + wsHandler: wsProxy, + auth: auth, + } } // NewOAuthProxy creates a new instance of OOuthProxy from the options provided @@ -201,7 +206,13 @@ func NewOAuthProxy(opts *Options, validator func(string) bool) *OAuthProxy { } logger.Printf("mapping path %q => file system %q", path, u.Path) proxy := NewFileServer(path, u.Path) - serveMux.Handle(path, &UpstreamProxy{path, proxy, nil, nil}) + uProxy := UpstreamProxy{ + upstream: path, + handler: proxy, + wsHandler: nil, + auth: nil, + } + serveMux.Handle(path, &uProxy) default: panic(fmt.Sprintf("unknown upstream protocol %s", u.Scheme)) } diff --git a/oauthproxy_test.go b/oauthproxy_test.go index 35ed59a..f114c7e 100644 --- a/oauthproxy_test.go +++ b/oauthproxy_test.go @@ -163,9 +163,9 @@ func TestEncodedSlashes(t *testing.T) { func TestRobotsTxt(t *testing.T) { opts := NewOptions() - opts.ClientID = "bazquux" - opts.ClientSecret = "foobar" - opts.CookieSecret = "xyzzyplugh" + opts.ClientID = "asdlkjx" + opts.ClientSecret = "alkgks" + opts.CookieSecret = "asdkugkj" opts.Validate() proxy := NewOAuthProxy(opts, func(string) bool { return true }) @@ -178,9 +178,9 @@ func TestRobotsTxt(t *testing.T) { func TestIsValidRedirect(t *testing.T) { opts := NewOptions() - opts.ClientID = "bazquux" - opts.ClientSecret = "foobar" - opts.CookieSecret = "xyzzyplugh" + opts.ClientID = "skdlfj" + opts.ClientSecret = "fgkdsgj" + opts.CookieSecret = "ljgiogbj" // Should match domains that are exactly foo.bar and any subdomain of bar.foo opts.WhitelistDomains = []string{"foo.bar", ".bar.foo"} opts.Validate() @@ -298,8 +298,8 @@ func TestBasicAuthPassword(t *testing.T) { // The CookieSecret must be 32 bytes in order to create the AES // cipher. opts.CookieSecret = "xyzzyplughxyzzyplughxyzzyplughxp" - opts.ClientID = "bazquux" - opts.ClientSecret = "foobar" + opts.ClientID = "dlgkj" + opts.ClientSecret = "alkgret" opts.CookieSecure = false opts.PassBasicAuth = true opts.PassUserHeaders = true @@ -392,8 +392,8 @@ func NewPassAccessTokenTest(opts PassAccessTokenTestOptions) *PassAccessTokenTes // The CookieSecret must be 32 bytes in order to create the AES // cipher. t.opts.CookieSecret = "xyzzyplughxyzzyplughxyzzyplughxp" - t.opts.ClientID = "bazquux" - t.opts.ClientSecret = "foobar" + t.opts.ClientID = "slgkj" + t.opts.ClientSecret = "gfjgojl" t.opts.CookieSecure = false t.opts.PassAccessToken = opts.PassAccessToken t.opts.Validate() @@ -518,9 +518,9 @@ func NewSignInPageTest(skipProvider bool) *SignInPageTest { var sipTest SignInPageTest sipTest.opts = NewOptions() - sipTest.opts.CookieSecret = "foobar" - sipTest.opts.ClientID = "bazquux" - sipTest.opts.ClientSecret = "xyzzyplugh" + sipTest.opts.CookieSecret = "adklsj2" + sipTest.opts.ClientID = "lkdgj" + sipTest.opts.ClientSecret = "sgiufgoi" sipTest.opts.SkipProviderButton = skipProvider sipTest.opts.Validate() @@ -624,8 +624,8 @@ func NewProcessCookieTest(opts ProcessCookieTestOpts, modifiers ...OptionsModifi for _, modifier := range modifiers { modifier(pcTest.opts) } - pcTest.opts.ClientID = "bazquux" - pcTest.opts.ClientSecret = "xyzzyplugh" + pcTest.opts.ClientID = "asdfljk" + pcTest.opts.ClientSecret = "lkjfdsig" pcTest.opts.CookieSecret = "0123456789abcdefabcd" // First, set the CookieRefresh option so proxy.AesCipher is created, // needed to encrypt the access_token. @@ -860,9 +860,9 @@ func TestAuthSkippedForPreflightRequests(t *testing.T) { opts := NewOptions() opts.Upstreams = append(opts.Upstreams, upstream.URL) - opts.ClientID = "bazquux" - opts.ClientSecret = "foobar" - opts.CookieSecret = "xyzzyplugh" + opts.ClientID = "aljsal" + opts.ClientSecret = "jglkfsdgj" + opts.CookieSecret = "dkfjgdls" opts.SkipAuthPreflight = true opts.Validate() @@ -999,8 +999,8 @@ func TestNoRequestSignature(t *testing.T) { func TestRequestSignatureGetRequest(t *testing.T) { st := NewSignatureTest() defer st.Close() - st.opts.SignatureKey = "sha1:foobar" - st.MakeRequestWithExpectedKey("GET", "", "foobar") + st.opts.SignatureKey = "sha1:7d9e1aa87a5954e6f9fc59266b3af9d7c35fda2d" + st.MakeRequestWithExpectedKey("GET", "", "7d9e1aa87a5954e6f9fc59266b3af9d7c35fda2d") assert.Equal(t, 200, st.rw.Code) assert.Equal(t, st.rw.Body.String(), "signatures match") } @@ -1008,9 +1008,9 @@ func TestRequestSignatureGetRequest(t *testing.T) { func TestRequestSignaturePostRequest(t *testing.T) { st := NewSignatureTest() defer st.Close() - st.opts.SignatureKey = "sha1:foobar" + st.opts.SignatureKey = "sha1:d90df39e2d19282840252612dd7c81421a372f61" payload := `{ "hello": "world!" }` - st.MakeRequestWithExpectedKey("POST", payload, "foobar") + st.MakeRequestWithExpectedKey("POST", payload, "d90df39e2d19282840252612dd7c81421a372f61") assert.Equal(t, 200, st.rw.Code) assert.Equal(t, st.rw.Body.String(), "signatures match") } @@ -1056,9 +1056,9 @@ type ajaxRequestTest struct { func newAjaxRequestTest() *ajaxRequestTest { test := &ajaxRequestTest{} test.opts = NewOptions() - test.opts.CookieSecret = "foobar" - test.opts.ClientID = "bazquux" - test.opts.ClientSecret = "xyzzyplugh" + test.opts.CookieSecret = "sdflsw" + test.opts.ClientID = "gkljfdl" + test.opts.ClientSecret = "sdflkjs" test.opts.Validate() test.proxy = NewOAuthProxy(test.opts, func(email string) bool { return true diff --git a/options.go b/options.go index 8c73eb9..1f438b5 100644 --- a/options.go +++ b/options.go @@ -454,7 +454,7 @@ func parseSignatureKey(o *Options, msgs []string) []string { return append(msgs, "unsupported signature hash algorithm: "+ o.SignatureKey) } - o.signatureData = &SignatureData{hash, secretKey} + o.signatureData = &SignatureData{hash: hash, key: secretKey} return msgs } From 5bcb998e6be341857d2c845a837a3c3c79dc63af Mon Sep 17 00:00:00 2001 From: Henry Jenkins Date: Sun, 23 Jun 2019 20:55:42 +0100 Subject: [PATCH 3/4] Update changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 69d071f..816fa97 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -62,7 +62,7 @@ - [#185](https://github.com/pusher/oauth2_proxy/pull/185) Fix an unsupported protocol scheme error during token validation when using the Azure provider (@jonas) - [#141](https://github.com/pusher/oauth2_proxy/pull/141) Check google group membership based on email address (@bchess) - Google Group membership is additionally checked via email address, allowing users outside a GSuite domain to be authorized. -- [#XX](https://github.com/pusher/outh2_proxy/pull/XX) Switch from gometalinter to golangci-lint +- [#198](https://github.com/pusher/outh2_proxy/pull/198) Switch from gometalinter to golangci-lint (@steakunderscore) # v3.2.0 From ce7e3840958fe3e2e62eab2386f25dd6e20f13b9 Mon Sep 17 00:00:00 2001 From: hjenkins Date: Mon, 1 Jul 2019 16:27:19 +0100 Subject: [PATCH 4/4] Remove TODO vetshadow as it's part of govet --- .golangci.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.golangci.yml b/.golangci.yml index 4dc2d94..a0658e1 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -3,7 +3,6 @@ run: linters: enable: - govet - # TODO: Not supported by golang-ci - vetshadow - golint - ineffassign - goconst