From 330f96ca637a651861917b13d4785b6f18440dd4 Mon Sep 17 00:00:00 2001 From: Rohith Jayawardene Date: Sun, 23 Apr 2017 16:48:38 +0100 Subject: [PATCH] Linting Errors (#209) - fixing up a number of warming found by the linters --- Makefile | 33 +++++++++++++++---------------- config.go | 18 +++++------------ cookies.go | 4 ++-- handlers.go | 10 +++++----- middleware.go | 6 +++--- middleware_test.go | 2 +- misc.go | 2 +- oauth.go | 2 +- server.go | 4 ++-- server_test.go | 4 ++-- store_boltdb.go | 2 +- user_context.go | 6 +----- utils.go | 48 +++++----------------------------------------- utils_test.go | 6 ------ 14 files changed, 44 insertions(+), 103 deletions(-) diff --git a/Makefile b/Makefile index c02bc1ad..4042d598 100644 --- a/Makefile +++ b/Makefile @@ -2,8 +2,7 @@ NAME=keycloak-proxy AUTHOR=gambol99 AUTHOR_EMAIL=gambol99@gmail.com REGISTRY=quay.io -GOVERSION ?= 1.8.0 -SUDO= +GOVERSION ?= 1.8.1 ROOT_DIR=${PWD} HARDWARE=$(shell uname -m) GIT_SHA=$(shell git --no-pager describe --always --dirty) @@ -27,39 +26,32 @@ version: build: @echo "--> Compiling the project" - mkdir -p bin + @mkdir -p bin godep go build -ldflags "${LFLAGS}" -o bin/${NAME} static: golang deps @echo "--> Compiling the static binary" - mkdir -p bin + @mkdir -p bin CGO_ENABLED=0 GOOS=linux godep go build -a -tags netgo -ldflags "-w ${LFLAGS}" -o bin/${NAME} docker-build: @echo "--> Compiling the project" - ${SUDO} docker run --rm -v ${ROOT_DIR}:/go/src/github.com/gambol99/keycloak-proxy \ - -w /go/src/github.com/gambol99/keycloak-proxy -e GOOS=linux golang:${GOVERSION} make static + docker run --rm \ + -v ${ROOT_DIR}:/go/src/github.com/gambol99/keycloak-proxy \ + -w /go/src/github.com/gambol99/keycloak-proxy \ + -e GOOS=linux golang:${GOVERSION} \ + make static docker-test: @echo "--> Running the docker test" - ${SUDO} docker run --rm -ti -p 3000:3000 \ + docker run --rm -ti -p 3000:3000 \ -v ${ROOT_DIR}/config.yml:/etc/keycloak/config.yml:ro \ -v ${ROOT_DIR}/tests:/opt/tests:ro \ ${REGISTRY}/${AUTHOR}/${NAME}:${VERSION} --config /etc/keycloak/config.yml docker: @echo "--> Building the docker image" - ${SUDO} docker build -t ${REGISTRY}/${AUTHOR}/${NAME}:${VERSION} . - -docker-release: - @echo "--> Building a release image" - @make static - @make docker - @docker push ${REGISTRY}/${AUTHOR}/${NAME}:${VERSION} - -docker-push: - @echo "--> Pushing the docker images to the registry" - ${SUDO} docker push ${REGISTRY}/${AUTHOR}/${NAME}:${VERSION} + docker build -t ${REGISTRY}/${AUTHOR}/${NAME}:${VERSION} . certs: @echo "--> Generating the root CA" @@ -112,6 +104,10 @@ gofmt: exit 1; \ fi +verify: + @echo "--> Linting the code" + @gometalinter --disable=errcheck --disable=gocyclo --disable=gas --disable=aligncheck + format: @echo "--> Running go fmt" @gofmt -s -w *.go @@ -132,6 +128,7 @@ cover: test: deps @echo "--> Running the tests" @godep go test -v + @$(MAKE) golang @$(MAKE) gofmt @$(MAKE) vet @$(MAKE) cover diff --git a/config.go b/config.go index a765cf31..c3b02ccb 100644 --- a/config.go +++ b/config.go @@ -28,9 +28,9 @@ import ( func newDefaultConfig() *Config { return &Config{ AccessTokenDuration: time.Duration(720) * time.Hour, - Tags: make(map[string]string, 0), - MatchClaims: make(map[string]string, 0), - Headers: make(map[string]string, 0), + Tags: make(map[string]string), + MatchClaims: make(map[string]string), + Headers: make(map[string]string), UpstreamTimeout: time.Duration(10) * time.Second, UpstreamKeepaliveTimeout: time.Duration(10) * time.Second, EnableAuthorizationHeader: true, @@ -154,18 +154,10 @@ func (r *Config) isValid() error { // hasCustomSignInPage checks if there is a custom sign in page func (r *Config) hasCustomSignInPage() bool { - if r.SignInPage != "" { - return true - } - - return false + return r.SignInPage != "" } // hasForbiddenPage checks if there is a custom forbidden page func (r *Config) hasCustomForbiddenPage() bool { - if r.ForbiddenPage != "" { - return true - } - - return false + return r.ForbiddenPage != "" } diff --git a/cookies.go b/cookies.go index 06d902c8..a852d1b3 100644 --- a/cookies.go +++ b/cookies.go @@ -61,10 +61,10 @@ func (r *oauthProxy) clearAllCookies(req *http.Request, w http.ResponseWriter) { // clearRefreshSessionCookie clears the session cookie func (r *oauthProxy) clearRefreshTokenCookie(req *http.Request, w http.ResponseWriter) { - r.dropCookie(w, req.Host, r.config.CookieRefreshName, "", time.Duration(-10*time.Hour)) + r.dropCookie(w, req.Host, r.config.CookieRefreshName, "", -10*time.Hour) } // clearAccessTokenCookie clears the session cookie func (r *oauthProxy) clearAccessTokenCookie(req *http.Request, w http.ResponseWriter) { - r.dropCookie(w, req.Host, r.config.CookieAccessName, "", time.Duration(-10*time.Hour)) + r.dropCookie(w, req.Host, r.config.CookieAccessName, "", -10*time.Hour) } diff --git a/handlers.go b/handlers.go index 45be01ff..76678494 100644 --- a/handlers.go +++ b/handlers.go @@ -126,7 +126,7 @@ func (r *oauthProxy) oauthAuthorizationHandler(cx echo.Context) error { // step: if we have a custom sign in page, lets display that if r.config.hasCustomSignInPage() { - model := make(map[string]string, 0) + model := make(map[string]string) model["redirect"] = authURL return cx.Render(http.StatusOK, path.Base(r.config.SignInPage), mergeMaps(model, r.config.Tags)) @@ -190,7 +190,7 @@ func (r *oauthProxy) oauthCallbackHandler(cx echo.Context) error { log.WithFields(log.Fields{ "email": identity.Email, "expires": identity.ExpiresAt.Format(time.RFC3339), - "duration": identity.ExpiresAt.Sub(time.Now()).String(), + "duration": time.Until(identity.ExpiresAt).String(), }).Infof("issuing access token for user, email: %s", identity.Email) // step: does the response has a refresh token and we are NOT ignore refresh tokens? @@ -218,11 +218,11 @@ func (r *oauthProxy) oauthCallbackHandler(cx echo.Context) error { if _, ident, err := parseToken(resp.RefreshToken); err != nil { r.dropRefreshTokenCookie(cx.Request(), cx.Response().Writer, encrypted, time.Duration(240)*time.Hour) } else { - r.dropRefreshTokenCookie(cx.Request(), cx.Response().Writer, encrypted, ident.ExpiresAt.Sub(time.Now())) + r.dropRefreshTokenCookie(cx.Request(), cx.Response().Writer, encrypted, time.Until(ident.ExpiresAt)) } } } else { - r.dropAccessTokenCookie(cx.Request(), cx.Response().Writer, token.Encode(), identity.ExpiresAt.Sub(time.Now())) + r.dropAccessTokenCookie(cx.Request(), cx.Response().Writer, token.Encode(), time.Until(identity.ExpiresAt)) } // step: decode the state variable @@ -277,7 +277,7 @@ func (r *oauthProxy) loginHandler(cx echo.Context) error { return "unable to decode the access token", http.StatusNotImplemented, err } - r.dropAccessTokenCookie(cx.Request(), cx.Response().Writer, token.AccessToken, identity.ExpiresAt.Sub(time.Now())) + r.dropAccessTokenCookie(cx.Request(), cx.Response().Writer, token.AccessToken, time.Until(identity.ExpiresAt)) cx.JSON(http.StatusOK, tokenResponse{ IDToken: token.IDToken, diff --git a/middleware.go b/middleware.go index 6127592b..282fdb8c 100644 --- a/middleware.go +++ b/middleware.go @@ -70,7 +70,7 @@ func (r *oauthProxy) loggingMiddleware() echo.MiddlewareFunc { return func(cx echo.Context) error { start := time.Now() next(cx) - latency := time.Now().Sub(start) + latency := time.Since(start) addr := cx.RealIP() log.WithFields(log.Fields{ "client_ip": addr, @@ -236,7 +236,7 @@ func (r *oauthProxy) authenticationMiddleware(resource *Resource) echo.Middlewar // admissionMiddleware is responsible checking the access token against the protected resource func (r *oauthProxy) admissionMiddleware(resource *Resource) echo.MiddlewareFunc { - claimMatches := make(map[string]*regexp.Regexp, 0) + claimMatches := make(map[string]*regexp.Regexp) for k, v := range r.config.MatchClaims { claimMatches[k] = regexp.MustCompile(v) } @@ -318,7 +318,7 @@ func (r *oauthProxy) admissionMiddleware(resource *Resource) echo.MiddlewareFunc log.WithFields(log.Fields{ "access": "permitted", "email": user.email, - "expires": user.expiresAt.Sub(time.Now()).String(), + "expires": time.Until(user.expiresAt).String(), "resource": resource.URL, }).Debugf("access permitted to resource") diff --git a/middleware_test.go b/middleware_test.go index 1a8bed63..49cac937 100644 --- a/middleware_test.go +++ b/middleware_test.go @@ -89,7 +89,7 @@ func newFakeProxy(c *Config) *fakeProxy { panic("failed to recreate the openid client, error: " + err.Error()) } - return &fakeProxy{c, auth, proxy, service, make(map[string]*http.Cookie, 0)} + return &fakeProxy{c, auth, proxy, service, make(map[string]*http.Cookie)} } // RunTests performs a series of requests against a fake proxy service diff --git a/misc.go b/misc.go index cf2d578e..4f238bf1 100644 --- a/misc.go +++ b/misc.go @@ -85,7 +85,7 @@ func (r *oauthProxy) getAccessCookieExpiration(token jose.JWT, refresh string) t // refresh token duration := r.config.AccessTokenDuration if _, ident, err := parseToken(refresh); err == nil { - duration = ident.ExpiresAt.Sub(time.Now()) + duration = time.Until(ident.ExpiresAt) } return duration diff --git a/oauth.go b/oauth.go index c965856f..a9f39065 100644 --- a/oauth.go +++ b/oauth.go @@ -104,7 +104,7 @@ func getUserinfo(client *oauth2.Client, endpoint string, token string) (jose.Cla return nil, err } var claims jose.Claims - if err := json.Unmarshal([]byte(content), &claims); err != nil { + if err := json.Unmarshal(content, &claims); err != nil { return nil, newAPIError("unable to decode response", resp.StatusCode) } diff --git a/server.go b/server.go index 223d88d9..60388c51 100644 --- a/server.go +++ b/server.go @@ -242,7 +242,7 @@ func (r *oauthProxy) createForwardingProxy() error { // @NOTES, somewhat annoying but goproxy hands back a nil response on proxy client errors if resp != nil && r.config.EnableLogging { start := ctx.UserData.(time.Time) - latency := time.Now().Sub(start) + latency := time.Since(start) log.WithFields(log.Fields{ "method": resp.Request.Method, @@ -306,7 +306,7 @@ func (r *oauthProxy) Run() error { } httpsvc := &http.Server{ Addr: r.config.ListenHTTP, - Handler: http.Handler(r.router), + Handler: r.router, } go func() { if err := httpsvc.Serve(httpListener); err != nil { diff --git a/server_test.go b/server_test.go index 0d1e1e31..7ae20bdc 100644 --- a/server_test.go +++ b/server_test.go @@ -208,7 +208,7 @@ func newTestProxyService(config *Config) (*oauthProxy, *fakeAuthServer, string) func newFakeHTTPRequest(method, path string) *http.Request { return &http.Request{ Method: method, - Header: make(map[string][]string, 0), + Header: make(map[string][]string), Host: "127.0.0.1", URL: &url.URL{ Scheme: "http", @@ -319,7 +319,7 @@ type fakeToken struct { } func newTestToken(issuer string) *fakeToken { - claims := make(jose.Claims, 0) + claims := make(jose.Claims) for k, v := range defaultTestTokenClaims { claims[k] = v } diff --git a/store_boltdb.go b/store_boltdb.go index 879374cc..92181684 100644 --- a/store_boltdb.go +++ b/store_boltdb.go @@ -45,7 +45,7 @@ func newBoltDBStore(location *url.URL) (storage, error) { log.Infof("creating the bolddb store, file: %s", path) db, err := bolt.Open(path, 0600, &bolt.Options{ - Timeout: time.Duration(10 * time.Second), + Timeout: 10 * time.Second, }) if err != nil { return nil, err diff --git a/user_context.go b/user_context.go index 58807e7f..4d1bbad5 100644 --- a/user_context.go +++ b/user_context.go @@ -80,11 +80,7 @@ func extractIdentity(token jose.JWT) (*userContext, error) { // isAudience checks the audience func (r *userContext) isAudience(aud string) bool { - if r.audience == aud { - return true - } - - return false + return r.audience == aud } // getRoles returns a list of roles diff --git a/utils.go b/utils.go index b1318919..b3d4e7fe 100644 --- a/utils.go +++ b/utils.go @@ -63,8 +63,7 @@ var ( ) var ( - httpMethodRegex = regexp.MustCompile("^(ANY|GET|POST|DELETE|PATCH|HEAD|PUT|TRACE)$") - symbolsFilter = regexp.MustCompilePOSIX("[_$><\\[\\].,\\+-/'%^&*()!\\\\]+") + symbolsFilter = regexp.MustCompilePOSIX("[_$><\\[\\].,\\+-/'%^&*()!\\\\]+") ) // readConfigFile reads and parses the configuration file @@ -218,7 +217,7 @@ func newOpenIDClient(cfg *Config) (*oidc.Client, oidc.ProviderConfig, *http.Clie // decodeKeyPairs converts a list of strings (key=pair) to a map func decodeKeyPairs(list []string) (map[string]string, error) { - kp := make(map[string]string, 0) + kp := make(map[string]string) for _, x := range list { items := strings.Split(x, "=") @@ -251,34 +250,6 @@ func defaultTo(v, d string) string { return d } -// cloneTLSConfig clones the tls configuration -func cloneTLSConfig(cfg *tls.Config) *tls.Config { - if cfg == nil { - return &tls.Config{} - } - return &tls.Config{ - Rand: cfg.Rand, - Time: cfg.Time, - Certificates: cfg.Certificates, - NameToCertificate: cfg.NameToCertificate, - GetCertificate: cfg.GetCertificate, - RootCAs: cfg.RootCAs, - NextProtos: cfg.NextProtos, - ServerName: cfg.ServerName, - ClientAuth: cfg.ClientAuth, - ClientCAs: cfg.ClientCAs, - InsecureSkipVerify: cfg.InsecureSkipVerify, - CipherSuites: cfg.CipherSuites, - PreferServerCipherSuites: cfg.PreferServerCipherSuites, - SessionTicketsDisabled: cfg.SessionTicketsDisabled, - SessionTicketKey: cfg.SessionTicketKey, - ClientSessionCache: cfg.ClientSessionCache, - MinVersion: cfg.MinVersion, - MaxVersion: cfg.MaxVersion, - CurvePreferences: cfg.CurvePreferences, - } -} - // fileExists check if a file exists func fileExists(filename string) bool { if _, err := os.Stat(filename); err != nil { @@ -338,22 +309,13 @@ func tryDialEndpoint(location *url.URL) (net.Conn, error) { // isUpgradedConnection checks to see if the request is requesting func isUpgradedConnection(req *http.Request) bool { - if req.Header.Get(headerUpgrade) != "" { - return true - } - - return false + return req.Header.Get(headerUpgrade) != "" } // transferBytes transfers bytes between the sink and source func transferBytes(src io.Reader, dest io.Writer, wg *sync.WaitGroup) (int64, error) { defer wg.Done() - copied, err := io.Copy(dest, src) - if err != nil { - return copied, err - } - - return copied, nil + return io.Copy(dest, src) } // tryUpdateConnection attempt to upgrade the connection to a http pdy stream @@ -473,7 +435,7 @@ func getWithin(expires time.Time, within float64) time.Duration { if left <= 0 { return time.Duration(0) } - seconds := int(float64(left * within)) + seconds := int(left * within) return time.Duration(seconds) * time.Second } diff --git a/utils_test.go b/utils_test.go index f4339134..3334b2aa 100644 --- a/utils_test.go +++ b/utils_test.go @@ -17,7 +17,6 @@ package main import ( "bytes" - "crypto/tls" "fmt" "io/ioutil" "net/http" @@ -247,11 +246,6 @@ func BenchmarkContainsSubString(t *testing.B) { } } -func TestCloneTLSConfig(t *testing.T) { - assert.NotNil(t, cloneTLSConfig(nil)) - assert.NotNil(t, cloneTLSConfig(&tls.Config{})) -} - func TestDialAddress(t *testing.T) { assert.Equal(t, dialAddress(getFakeURL("http://127.0.0.1")), "127.0.0.1:80") assert.Equal(t, dialAddress(getFakeURL("https://127.0.0.1")), "127.0.0.1:443")