Linting

* abide by modern go linting standards (golangci)
* include `.golangci.yml` config to (optionally) carry out automated linting check on PRs
* does not change behavior

NOTE: upgrading deprecated prometheus middleware is not carried out in this PR

Signed-off-by: Frederic BIDON <[email protected]>

.golangci.yml

@@ -0,0 +1,23 @@
+linters-settings:
+  govet:
+    check-shadowing: true
+  golint:
+    min-confidence: 0
+  gocyclo:
+    min-complexity: 60
+  maligned:
+    suggest-new: true
+  dupl:
+    threshold: 100
+  goconst:
+    min-len: 2
+    min-occurrences: 2
+
+linters:
+  enable-all: true
+  disable:
+    - maligned
+    - unparam
+    - lll
+    - gochecknoinits
+    - gochecknoglobals

cli.go

@@ -26,6 +26,8 @@ import (
 	"github.com/urfave/cli"
 )
 
+const durationType = "time.Duration"
+
 // newOauthProxyApp creates a new cli application and runs it
 func newOauthProxyApp() *cli.App {
 	config := newDefaultConfig()
@@ -76,7 +78,7 @@ func newOauthProxyApp() *cli.App {
 		}
 
 		// step: setup the termination signals
-		signalChannel := make(chan os.Signal)
+		signalChannel := make(chan os.Signal, 1)
 		signal.Notify(signalChannel, syscall.SIGHUP, syscall.SIGINT, syscall.SIGTERM, syscall.SIGQUIT)
 		<-signalChannel
 
@@ -136,7 +138,7 @@ func getCommandLineOptions() []cli.Flag {
 			})
 		case reflect.Int64:
 			switch t.String() {
-			case "time.Duration":
+			case durationType:
 				dv := reflect.ValueOf(defaults).Elem().FieldByName(field.Name).Int()
 				flags = append(flags, cli.DurationFlag{
 					Name:  optName,
@@ -180,7 +182,7 @@ func parseCLIOptions(cx *cli.Context, config *Config) (err error) {
 				reflect.ValueOf(config).Elem().FieldByName(field.Name).Set(reflect.ValueOf(cx.Int(name)))
 			case reflect.Int64:
 				switch field.Type.String() {
-				case "time.Duration":
+				case durationType:
 					reflect.ValueOf(config).Elem().FieldByName(field.Name).SetInt(int64(cx.Duration(name)))
 				default:
 					reflect.ValueOf(config).Elem().FieldByName(field.Name).SetInt(cx.Int64(name))

cli_test.go

@@ -37,8 +37,10 @@ func TestReadOptions(t *testing.T) {
 	c := cli.NewApp()
 	c.Flags = getCommandLineOptions()
 	c.Action = func(cx *cli.Context) error {
-		parseCLIOptions(cx, &Config{})
+		ero := parseCLIOptions(cx, &Config{})
+		assert.NoError(t, ero)
 		return nil
 	}
-	c.Run([]string{""})
+	err := c.Run([]string{""})
+	assert.NoError(t, err)
 }

config.go

@@ -35,8 +35,8 @@ func newDefaultConfig() *Config {
 
 	return &Config{
 		AccessTokenDuration:           time.Duration(720) * time.Hour,
-		CookieAccessName:              "kc-access",
-		CookieRefreshName:             "kc-state",
+		CookieAccessName:              accessCookie,
+		CookieRefreshName:             refreshCookie,
 		EnableAuthorizationCookies:    true,
 		EnableAuthorizationHeader:     true,
 		EnableDefaultDeny:             true,
@@ -61,7 +61,7 @@ func newDefaultConfig() *Config {
 		ServerWriteTimeout:            10 * time.Second,
 		SkipOpenIDProviderTLSVerify:   false,
 		SkipUpstreamTLSVerify:         true,
-		Tags:                          make(map[string]string, 0),
+		Tags:                          make(map[string]string),
 		UpstreamExpectContinueTimeout: 10 * time.Second,
 		UpstreamKeepaliveTimeout:      10 * time.Second,
 		UpstreamKeepalives:            true,

cookies.go

@@ -22,7 +22,7 @@ import (
 	"strings"
 	"time"
 
-	"github.com/satori/go.uuid"
+	uuid "github.com/satori/go.uuid"
 )
 
 // dropCookie drops a cookie into the response
@@ -51,64 +51,56 @@ func (r *oauthProxy) dropCookie(w http.ResponseWriter, host, name, value string,
 func (r *oauthProxy) getMaxCookieChunkLength(req *http.Request, cookieName string) int {
 	maxCookieChunkLength := 4069 - len(cookieName)
 	if r.config.CookieDomain != "" {
-		maxCookieChunkLength = maxCookieChunkLength - len(r.config.CookieDomain)
+		maxCookieChunkLength -= len(r.config.CookieDomain)
 	} else {
-		maxCookieChunkLength = maxCookieChunkLength - len(strings.Split(req.Host, ":")[0])
+		maxCookieChunkLength -= len(strings.Split(req.Host, ":")[0])
 	}
 	if r.config.HTTPOnlyCookie {
-		maxCookieChunkLength = maxCookieChunkLength - len("HttpOnly; ")
+		maxCookieChunkLength -= len("HttpOnly; ")
 	}
 	if !r.config.EnableSessionCookies {
-		maxCookieChunkLength = maxCookieChunkLength - len("Expires=Mon, 02 Jan 2006 03:04:05 MST; ")
+		maxCookieChunkLength -= len("Expires=Mon, 02 Jan 2006 03:04:05 MST; ")
 	}
 	if r.config.SecureCookie {
-		maxCookieChunkLength = maxCookieChunkLength - len("Secure")
+		maxCookieChunkLength -= len("Secure")
 	}
 	return maxCookieChunkLength
 }
 
-// dropAccessTokenCookie drops a access token cookie into the response
-func (r *oauthProxy) dropAccessTokenCookie(req *http.Request, w http.ResponseWriter, value string, duration time.Duration) {
-	maxCookieChunkLength := r.getMaxCookieChunkLength(req, r.config.CookieAccessName)
+// dropCookieWithChunks drops a cookie from the response, taking into account possible chunks
+func (r *oauthProxy) dropCookieWithChunks(req *http.Request, w http.ResponseWriter, name, value string, duration time.Duration) {
+	maxCookieChunkLength := r.getMaxCookieChunkLength(req, name)
 	if len(value) <= maxCookieChunkLength {
-		r.dropCookie(w, req.Host, r.config.CookieAccessName, value, duration)
+		r.dropCookie(w, req.Host, name, value, duration)
 	} else {
 		// write divided cookies because payload is too long for single cookie
-		r.dropCookie(w, req.Host, r.config.CookieAccessName, value[0:maxCookieChunkLength], duration)
+		r.dropCookie(w, req.Host, name, value[0:maxCookieChunkLength], duration)
 		for i := maxCookieChunkLength; i < len(value); i += maxCookieChunkLength {
 			end := i + maxCookieChunkLength
 			if end > len(value) {
 				end = len(value)
 			}
-			r.dropCookie(w, req.Host, r.config.CookieAccessName+"-"+strconv.Itoa(i/maxCookieChunkLength), value[i:end], duration)
+			r.dropCookie(w, req.Host, name+"-"+strconv.Itoa(i/maxCookieChunkLength), value[i:end], duration)
 		}
 	}
 }
 
-// dropRefreshTokenCookie drops a refresh token cookie into the response
+// dropAccessTokenCookie drops a access token cookie from the response
+func (r *oauthProxy) dropAccessTokenCookie(req *http.Request, w http.ResponseWriter, value string, duration time.Duration) {
+	r.dropCookieWithChunks(req, w, r.config.CookieAccessName, value, duration)
+}
+
+// dropRefreshTokenCookie drops a refresh token cookie from the response
 func (r *oauthProxy) dropRefreshTokenCookie(req *http.Request, w http.ResponseWriter, value string, duration time.Duration) {
-	maxCookieChunkLength := r.getMaxCookieChunkLength(req, r.config.CookieRefreshName)
-	if len(value) <= maxCookieChunkLength {
-		r.dropCookie(w, req.Host, r.config.CookieRefreshName, value, duration)
-	} else {
-		// write divided cookies because payload is too long for single cookie
-		r.dropCookie(w, req.Host, r.config.CookieRefreshName, value[0:maxCookieChunkLength], duration)
-		for i := maxCookieChunkLength; i < len(value); i += maxCookieChunkLength {
-			end := i + maxCookieChunkLength
-			if end > len(value) {
-				end = len(value)
-			}
-			r.dropCookie(w, req.Host, r.config.CookieRefreshName+"-"+strconv.Itoa(i/maxCookieChunkLength), value[i:end], duration)
-		}
-	}
+	r.dropCookieWithChunks(req, w, r.config.CookieRefreshName, value, duration)
 }
 
-// dropStateParameterCookie drops a state parameter cookie into the response
+// writeStateParameterCookie sets a state parameter cookie into the response
 func (r *oauthProxy) writeStateParameterCookie(req *http.Request, w http.ResponseWriter) string {
 	uuid := uuid.NewV4().String()
 	requestURI := base64.StdEncoding.EncodeToString([]byte(req.URL.RequestURI()))
-	r.dropCookie(w, req.Host, "request_uri", requestURI, 0)
-	r.dropCookie(w, req.Host, "OAuth_Token_Request_State", uuid, 0)
+	r.dropCookie(w, req.Host, requestURICookie, requestURI, 0)
+	r.dropCookie(w, req.Host, requestStateCookie, uuid, 0)
 	return uuid
 }
 

cookies_test.go

@@ -32,7 +32,7 @@ func TestCookieDomainHostHeader(t *testing.T) {
 
 	var cookie *http.Cookie
 	for _, c := range resp.Cookies() {
-		if c.Name == "kc-access" {
+		if c.Name == accessCookie {
 			cookie = c
 		}
 	}
@@ -49,7 +49,7 @@ func TestCookieDomain(t *testing.T) {
 
 	var cookie *http.Cookie
 	for _, c := range resp.Cookies() {
-		if c.Name == "kc-access" {
+		if c.Name == accessCookie {
 			cookie = c
 		}
 	}
@@ -101,7 +101,7 @@ func TestDropRefreshCookie(t *testing.T) {
 	p.dropRefreshTokenCookie(req, resp, "test", 0)
 
 	assert.Equal(t, resp.Header().Get("Set-Cookie"),
-		"kc-state=test; Path=/; Domain=127.0.0.1",
+		refreshCookie+"=test; Path=/; Domain=127.0.0.1",
 		"we have not set the cookie, headers: %v", resp.Header())
 }
 
@@ -146,7 +146,7 @@ func TestClearAccessTokenCookie(t *testing.T) {
 	resp := httptest.NewRecorder()
 	p.clearAccessTokenCookie(req, resp)
 	assert.Contains(t, resp.Header().Get("Set-Cookie"),
-		"kc-access=; Path=/; Domain=127.0.0.1; Expires=",
+		accessCookie+"=; Path=/; Domain=127.0.0.1; Expires=",
 		"we have not cleared the, headers: %v", resp.Header())
 }
 
@@ -156,7 +156,7 @@ func TestClearRefreshAccessTokenCookie(t *testing.T) {
 	resp := httptest.NewRecorder()
 	p.clearRefreshTokenCookie(req, resp)
 	assert.Contains(t, resp.Header().Get("Set-Cookie"),
-		"kc-state=; Path=/; Domain=127.0.0.1; Expires=",
+		refreshCookie+"=; Path=/; Domain=127.0.0.1; Expires=",
 		"we have not cleared the, headers: %v", resp.Header())
 }
 
@@ -166,7 +166,7 @@ func TestClearAllCookies(t *testing.T) {
 	resp := httptest.NewRecorder()
 	p.clearAllCookies(req, resp)
 	assert.Contains(t, resp.Header().Get("Set-Cookie"),
-		"kc-access=; Path=/; Domain=127.0.0.1; Expires=",
+		accessCookie+"=; Path=/; Domain=127.0.0.1; Expires=",
 		"we have not cleared the, headers: %v", resp.Header())
 }
 

doc.go

@@ -33,17 +33,17 @@ var (
 	version  = ""
 )
 
+type contextKey int8
+
 const (
 	prog        = "keycloak-gatekeeper"
 	author      = "Keycloak"
 	email       = "[email protected]"
 	description = "is a proxy using the keycloak service for auth and authorization"
 
 	authorizationHeader = "Authorization"
-	contextScopeName    = "context.scope.name"
 	envPrefix           = "PROXY_"
 	headerUpgrade       = "Upgrade"
-	httpSchema          = "http"
 	versionHeader       = "X-Auth-Proxy-Version"
 
 	authorizationURL = "/authorize"
@@ -62,6 +62,17 @@ const (
 	claimResourceAccess = "resource_access"
 	claimResourceRoles  = "roles"
 	claimGroups         = "groups"
+
+	accessCookie       = "kc-access"
+	refreshCookie      = "kc-state"
+	requestURICookie   = "request_uri"
+	requestStateCookie = "OAuth_Token_Request_State"
+	unsecureScheme     = "http"
+	secureScheme       = "https"
+	anyMethod          = "ANY"
+
+	_ contextKey = iota
+	contextScopeName
 )
 
 const (

handlers.go

@@ -23,6 +23,7 @@ import (
 	"fmt"
 	"io/ioutil"
 	"net"
+
 	"net/http"
 	"net/http/pprof"
 	"net/url"
@@ -43,9 +44,9 @@ func (r *oauthProxy) getRedirectionURL(w http.ResponseWriter, req *http.Request)
 	case "":
 		// need to determine the scheme, cx.Request.URL.Scheme doesn't have it, best way is to default
 		// and then check for TLS
-		scheme := "http"
+		scheme := unsecureScheme
 		if req.TLS != nil {
-			scheme = "https"
+			scheme = secureScheme
 		}
 		// @QUESTION: should I use the X-Forwarded-<header>?? ..
 		redirect = fmt.Sprintf("%s://%s",
@@ -55,9 +56,9 @@ func (r *oauthProxy) getRedirectionURL(w http.ResponseWriter, req *http.Request)
 		redirect = r.config.RedirectionURL
 	}
 
-	state, _ := req.Cookie("OAuth_Token_Request_State")
+	state, _ := req.Cookie(requestStateCookie)
 	if state != nil && req.URL.Query().Get("state") != state.Value {
-		r.log.Error("State parameter mismatch")
+		r.log.Error("state parameter mismatch")
 		w.WriteHeader(http.StatusForbidden)
 		return ""
 	}
@@ -94,7 +95,7 @@ func (r *oauthProxy) oauthAuthorizationHandler(w http.ResponseWriter, req *http.
 		model := make(map[string]string)
 		model["redirect"] = authURL
 		w.WriteHeader(http.StatusOK)
-		r.Render(w, path.Base(r.config.SignInPage), mergeMaps(model, r.config.Tags))
+		_ = r.Render(w, path.Base(r.config.SignInPage), mergeMaps(model, r.config.Tags))
 
 		return
 	}
@@ -204,7 +205,7 @@ func (r *oauthProxy) oauthCallbackHandler(w http.ResponseWriter, req *http.Reque
 	// step: decode the request variable
 	redirectURI := "/"
 	if req.URL.Query().Get("state") != "" {
-		if encodedRequestURI, _ := req.Cookie("request_uri"); encodedRequestURI != nil {
+		if encodedRequestURI, _ := req.Cookie(requestURICookie); encodedRequestURI != nil {
 			decoded, _ := base64.StdEncoding.DecodeString(encodedRequestURI.Value)
 			redirectURI = string(decoded)
 		}
@@ -422,18 +423,19 @@ func (r *oauthProxy) tokenHandler(w http.ResponseWriter, req *http.Request) {
 		return
 	}
 	w.Header().Set("Content-Type", "application/json")
-	w.Write(user.token.Payload)
+	_, _ = w.Write(user.token.Payload)
 }
 
 // healthHandler is a health check handler for the service
 func (r *oauthProxy) healthHandler(w http.ResponseWriter, req *http.Request) {
 	w.Header().Set(versionHeader, getVersion())
 	w.WriteHeader(http.StatusOK)
-	w.Write([]byte("OK\n"))
+	_, _ = w.Write([]byte("OK\n"))
 }
 
 // debugHandler is responsible for providing the pprof
 func (r *oauthProxy) debugHandler(w http.ResponseWriter, req *http.Request) {
+	const symbolProfile = "symbol"
 	name := chi.URLParam(req, "name")
 	switch req.Method {
 	case http.MethodGet:
@@ -452,14 +454,14 @@ func (r *oauthProxy) debugHandler(w http.ResponseWriter, req *http.Request) {
 			pprof.Profile(w, req)
 		case "trace":
 			pprof.Trace(w, req)
-		case "symbol":
+		case symbolProfile:
 			pprof.Symbol(w, req)
 		default:
 			w.WriteHeader(http.StatusNotFound)
 		}
 	case http.MethodPost:
 		switch name {
-		case "symbol":
+		case symbolProfile:
 			pprof.Symbol(w, req)
 		default:
 			w.WriteHeader(http.StatusNotFound)
@@ -497,5 +499,5 @@ func (r *oauthProxy) retrieveRefreshToken(req *http.Request, user *userContext)
 
 func methodNotAllowHandlder(w http.ResponseWriter, req *http.Request) {
 	w.WriteHeader(http.StatusMethodNotAllowed)
-	w.Write(nil)
+	_, _ = w.Write(nil)
 }

handlers_test.go

@@ -51,13 +51,13 @@ func TestExpirationHandler(t *testing.T) {
 		{
 			URI:          uri,
 			HasToken:     true,
-			Expires:      time.Duration(-48 * time.Hour),
+			Expires:      -48 * time.Hour,
 			ExpectedCode: http.StatusUnauthorized,
 		},
 		{
 			URI:          uri,
 			HasToken:     true,
-			Expires:      time.Duration(14 * time.Hour),
+			Expires:      14 * time.Hour,
 			ExpectedCode: http.StatusOK,
 		},
 	}

main.go

@@ -21,5 +21,5 @@ import (
 
 func main() {
 	app := newOauthProxyApp()
-	app.Run(os.Args)
+	_ = app.Run(os.Args)
 }