Fix linting issues (#197)

* Fix linting issues

* Linting updates

.github/workflows/build.yml

@@ -66,6 +66,6 @@ jobs:
       - name: golangci-lint
         uses: golangci/golangci-lint-action@v3
         with:
-          version: v1.33
+          version: v1.46
           args: "--out-${NO_FUTURE}format colored-line-number"
           github-token: "${{ secrets.GITHUB_TOKEN }}"

.golangci.yml

@@ -1,7 +1,7 @@
 linters-settings:
   govet:
     check-shadowing: true
-  golint:
+  revive:
     min-confidence: 0
   gocyclo:
     min-complexity: 64
@@ -20,6 +20,9 @@ linters-settings:
 linters:
   enable-all: true
   disable:
+    - thelper
+    - ireturn
+    - maintidx
     - wrapcheck
     - gosec
     - gocritic
@@ -31,6 +34,8 @@ linters:
     - paralleltest
     - exhaustive
     - exhaustivestruct
+    - exhaustruct
+    - tagliatelle
     - maligned
     - unparam
     - lll

Makefile

@@ -103,7 +103,7 @@ lint:
 	@which golangci-lint 2>/dev/null ; if [ $$? -eq 1 ]; then \
 		go get -u github.com/golangci/golangci-lint/cmd/golangci-lint; \
 	fi
-	@golint .
+	@golangci-lint run .
 
 gofmt:
 	@echo "--> Running gofmt check"

cli.go

@@ -88,8 +88,11 @@ func newOauthProxyApp() *cli.App {
 	return app
 }
 
-// getCommandLineOptions builds the command line options by reflecting the Config struct and extracting
-// the tagged information
+/*
+	getCommandLineOptions builds the command line options by reflecting
+	the Config struct and extracting the tagged information
+*/
+//nolint:cyclop
 func getCommandLineOptions() []cli.Flag {
 	defaults := newDefaultConfig()
 	var flags []cli.Flag
@@ -166,8 +169,12 @@ func getCommandLineOptions() []cli.Flag {
 	return flags
 }
 
-// parseCLIOptions parses the command line options and constructs a config object
-func parseCLIOptions(cx *cli.Context, config *Config) (err error) {
+/*
+	parseCLIOptions parses the command line options
+	and constructs a config object
+*/
+//nolint:cyclop
+func parseCLIOptions(cliCtx *cli.Context, config *Config) error {
 	// step: we can ignore these options in the Config struct
 	ignoredOptions := []string{"tag-data", "match-claims", "resources", "headers"}
 	// step: iterate the Config and grab command line options via reflection
@@ -181,53 +188,53 @@ func parseCLIOptions(cx *cli.Context, config *Config) (err error) {
 			continue
 		}
 
-		if cx.IsSet(name) {
+		if cliCtx.IsSet(name) {
 			switch field.Type.Kind() {
 			case reflect.Bool:
-				reflect.ValueOf(config).Elem().FieldByName(field.Name).SetBool(cx.Bool(name))
+				reflect.ValueOf(config).Elem().FieldByName(field.Name).SetBool(cliCtx.Bool(name))
 			case reflect.String:
-				reflect.ValueOf(config).Elem().FieldByName(field.Name).SetString(cx.String(name))
+				reflect.ValueOf(config).Elem().FieldByName(field.Name).SetString(cliCtx.String(name))
 			case reflect.Slice:
-				reflect.ValueOf(config).Elem().FieldByName(field.Name).Set(reflect.ValueOf(cx.StringSlice(name)))
+				reflect.ValueOf(config).Elem().FieldByName(field.Name).Set(reflect.ValueOf(cliCtx.StringSlice(name)))
 			case reflect.Int:
-				reflect.ValueOf(config).Elem().FieldByName(field.Name).Set(reflect.ValueOf(cx.Int(name)))
+				reflect.ValueOf(config).Elem().FieldByName(field.Name).Set(reflect.ValueOf(cliCtx.Int(name)))
 			case reflect.Int64:
 				switch field.Type.String() {
 				case durationType:
-					reflect.ValueOf(config).Elem().FieldByName(field.Name).SetInt(int64(cx.Duration(name)))
+					reflect.ValueOf(config).Elem().FieldByName(field.Name).SetInt(int64(cliCtx.Duration(name)))
 				default:
-					reflect.ValueOf(config).Elem().FieldByName(field.Name).SetInt(cx.Int64(name))
+					reflect.ValueOf(config).Elem().FieldByName(field.Name).SetInt(cliCtx.Int64(name))
 				}
 			}
 		}
 	}
 
-	if cx.IsSet("tag") {
-		tags, err := decodeKeyPairs(cx.StringSlice("tag"))
+	if cliCtx.IsSet("tag") {
+		tags, err := decodeKeyPairs(cliCtx.StringSlice("tag"))
 		if err != nil {
 			return err
 		}
 		mergeMaps(config.Tags, tags)
 	}
 
-	if cx.IsSet("match-claims") {
-		claims, err := decodeKeyPairs(cx.StringSlice("match-claims"))
+	if cliCtx.IsSet("match-claims") {
+		claims, err := decodeKeyPairs(cliCtx.StringSlice("match-claims"))
 		if err != nil {
 			return err
 		}
 		mergeMaps(config.MatchClaims, claims)
 	}
 
-	if cx.IsSet("headers") {
-		headers, err := decodeKeyPairs(cx.StringSlice("headers"))
+	if cliCtx.IsSet("headers") {
+		headers, err := decodeKeyPairs(cliCtx.StringSlice("headers"))
 		if err != nil {
 			return err
 		}
 		mergeMaps(config.Headers, headers)
 	}
 
-	if cx.IsSet("resources") {
-		for _, x := range cx.StringSlice("resources") {
+	if cliCtx.IsSet("resources") {
+		for _, x := range cliCtx.StringSlice("resources") {
 			resource, err := newResource().parse(x)
 			if err != nil {
 				return fmt.Errorf("invalid resource %s, %s", x, err)

common_test.go

@@ -218,7 +218,7 @@ func (f *fakeProxy) getServiceURL() string {
 }
 
 // RunTests performs a series of requests against a fake proxy service
-// nolint:gocyclo,funlen
+//nolint:gocyclo,funlen,cyclop
 func (f *fakeProxy) RunTests(t *testing.T, requests []fakeRequest) {
 	defer func() {
 		f.idp.Close()
@@ -801,14 +801,14 @@ func makeTestCodeFlowLogin(location string) (*http.Response, []*http.Cookie, err
 		}
 
 		// step: make the request
-		tr := &http.Transport{
+		transport := &http.Transport{
 			TLSClientConfig: &tls.Config{
 				//nolint:gas
 				InsecureSkipVerify: true,
 			},
 		}
 
-		resp, err = tr.RoundTrip(req)
+		resp, err = transport.RoundTrip(req)
 
 		if err != nil {
 			return nil, nil, err
@@ -861,8 +861,7 @@ func (f *fakeUpstreamService) ServeHTTP(wrt http.ResponseWriter, req *http.Reque
 		}).ServeHTTP(wrt, req)
 	} else {
 		wrt.Header().Set("Content-Type", "application/json")
-		wrt.WriteHeader(http.StatusOK)
-		content, _ := json.Marshal(&fakeUpstreamResponse{
+		content, err := json.Marshal(&fakeUpstreamResponse{
 			// r.RequestURI is what was received by the proxy.
 			// r.URL.String() is what is actually sent to the upstream service.
 			// KEYCLOAK-10864, KEYCLOAK-11276, KEYCLOAK-13315
@@ -871,6 +870,13 @@ func (f *fakeUpstreamService) ServeHTTP(wrt http.ResponseWriter, req *http.Reque
 			Address: req.RemoteAddr,
 			Headers: req.Header,
 		})
+
+		if err != nil {
+			wrt.WriteHeader(http.StatusInternalServerError)
+		} else {
+			wrt.WriteHeader(http.StatusOK)
+		}
+
 		_, _ = wrt.Write(content)
 	}
 }
@@ -1258,7 +1264,8 @@ func (r *fakeAuthServer) userInfoHandler(wrt http.ResponseWriter, req *http.Requ
 	})
 }
 
-func (r *fakeAuthServer) tokenHandler(w http.ResponseWriter, req *http.Request) {
+//nolint:cyclop
+func (r *fakeAuthServer) tokenHandler(writer http.ResponseWriter, req *http.Request) {
 	expires := time.Now().Add(r.expiration)
 	refreshExpires := time.Now().Add(2 * r.expiration)
 	token := newTestToken(r.getLocation())
@@ -1281,14 +1288,14 @@ func (r *fakeAuthServer) tokenHandler(w http.ResponseWriter, req *http.Request)
 	// sign the token with the private key
 	jwtAccess, err := token.getToken()
 	if err != nil {
-		w.WriteHeader(http.StatusInternalServerError)
+		writer.WriteHeader(http.StatusInternalServerError)
 		return
 	}
 
 	// sign the token with the private key
 	jwtRefresh, err := refreshToken.getToken()
 	if err != nil {
-		w.WriteHeader(http.StatusInternalServerError)
+		writer.WriteHeader(http.StatusInternalServerError)
 		return
 	}
 
@@ -1298,12 +1305,12 @@ func (r *fakeAuthServer) tokenHandler(w http.ResponseWriter, req *http.Request)
 		password := req.FormValue("password")
 
 		if username == "" || password == "" {
-			w.WriteHeader(http.StatusBadRequest)
+			writer.WriteHeader(http.StatusBadRequest)
 			return
 		}
 
 		if username == validUsername && password == validPassword {
-			renderJSON(http.StatusOK, w, req, tokenResponse{
+			renderJSON(http.StatusOK, writer, req, tokenResponse{
 				IDToken:      jwtAccess,
 				AccessToken:  jwtAccess,
 				RefreshToken: jwtRefresh,
@@ -1312,7 +1319,7 @@ func (r *fakeAuthServer) tokenHandler(w http.ResponseWriter, req *http.Request)
 			return
 		}
 
-		renderJSON(http.StatusUnauthorized, w, req, map[string]string{
+		renderJSON(http.StatusUnauthorized, writer, req, map[string]string{
 			"error":             "invalid_grant",
 			"error_description": "invalid user credentials",
 		})
@@ -1326,13 +1333,13 @@ func (r *fakeAuthServer) tokenHandler(w http.ResponseWriter, req *http.Request)
 			clientSecret = p
 
 			if clientID == "" || clientSecret == "" || !ok {
-				w.WriteHeader(http.StatusBadRequest)
+				writer.WriteHeader(http.StatusBadRequest)
 				return
 			}
 		}
 
 		if clientID == validUsername && clientSecret == validPassword {
-			renderJSON(http.StatusOK, w, req, tokenResponse{
+			renderJSON(http.StatusOK, writer, req, tokenResponse{
 				IDToken:      jwtAccess,
 				AccessToken:  jwtAccess,
 				RefreshToken: jwtRefresh,
@@ -1341,15 +1348,15 @@ func (r *fakeAuthServer) tokenHandler(w http.ResponseWriter, req *http.Request)
 			return
 		}
 
-		renderJSON(http.StatusUnauthorized, w, req, map[string]string{
+		renderJSON(http.StatusUnauthorized, writer, req, map[string]string{
 			"error":             "invalid_grant",
 			"error_description": "invalid client credentials",
 		})
 	case GrantTypeRefreshToken:
 		oldRefreshToken, err := jwt.ParseSigned(req.FormValue("refresh_token"))
 
 		if err != nil {
-			w.WriteHeader(http.StatusInternalServerError)
+			writer.WriteHeader(http.StatusInternalServerError)
 			return
 		}
 
@@ -1358,7 +1365,7 @@ func (r *fakeAuthServer) tokenHandler(w http.ResponseWriter, req *http.Request)
 		err = oldRefreshToken.UnsafeClaimsWithoutVerification(stdClaims)
 
 		if err != nil {
-			w.WriteHeader(http.StatusInternalServerError)
+			writer.WriteHeader(http.StatusInternalServerError)
 			return
 		}
 
@@ -1374,37 +1381,37 @@ func (r *fakeAuthServer) tokenHandler(w http.ResponseWriter, req *http.Request)
 			respBody, err := json.Marshal(expRefresh)
 
 			if err != nil {
-				w.WriteHeader(http.StatusInternalServerError)
+				writer.WriteHeader(http.StatusInternalServerError)
 				return
 			}
 
-			w.WriteHeader(http.StatusBadRequest)
-			_, _ = w.Write(respBody)
+			writer.WriteHeader(http.StatusBadRequest)
+			_, _ = writer.Write(respBody)
 
 			return
 		}
 
-		renderJSON(http.StatusOK, w, req, tokenResponse{
+		renderJSON(http.StatusOK, writer, req, tokenResponse{
 			IDToken:     jwtAccess,
 			AccessToken: jwtAccess,
 			ExpiresIn:   float64(expires.Second()),
 		})
 	case GrantTypeAuthCode:
-		renderJSON(http.StatusOK, w, req, tokenResponse{
+		renderJSON(http.StatusOK, writer, req, tokenResponse{
 			IDToken:      jwtAccess,
 			AccessToken:  jwtAccess,
 			RefreshToken: jwtRefresh,
 			ExpiresIn:    float64(expires.Second()),
 		})
 	case GrantTypeUmaTicket:
-		renderJSON(http.StatusOK, w, req, tokenResponse{
+		renderJSON(http.StatusOK, writer, req, tokenResponse{
 			IDToken:      jwtAccess,
 			AccessToken:  jwtAccess,
 			RefreshToken: jwtRefresh,
 			ExpiresIn:    float64(expires.Second()),
 		})
 	default:
-		w.WriteHeader(http.StatusBadRequest)
+		writer.WriteHeader(http.StatusBadRequest)
 	}
 }
 

config.go

@@ -203,6 +203,7 @@ func (r *Config) isSameSiteValid() error {
 	return nil
 }
 
+//nolint:cyclop
 func (r *Config) isTLSFilesValid() error {
 	if r.TLSCertificate != "" && r.TLSPrivateKey == "" {
 		return errors.New("you have not provided a private key")
@@ -237,6 +238,7 @@ func (r *Config) isTLSFilesValid() error {
 	return nil
 }
 
+//nolint:cyclop
 func (r *Config) isAdminTLSFilesValid() error {
 	if r.TLSAdminCertificate != "" && r.TLSAdminPrivateKey == "" {
 		return errors.New("you have not provided a private key for admin endpoint")
@@ -290,13 +292,13 @@ func (r *Config) isTLSMinValid() error {
 	switch strings.ToLower(r.TLSMinVersion) {
 	case "":
 		return fmt.Errorf("minimal TLS version should not be empty")
-	// nolint: goconst
+	//nolint: goconst
 	case "tlsv1.0":
-	// nolint: goconst
+	//nolint: goconst
 	case "tlsv1.1":
-	// nolint: goconst
+	//nolint: goconst
 	case "tlsv1.2":
-	// nolint: goconst
+	//nolint: goconst
 	case "tlsv1.3":
 	default:
 		return fmt.Errorf("invalid minimal TLS version specified")

config_test.go

@@ -474,6 +474,7 @@ func TestIsSameSiteValid(t *testing.T) {
 	}
 }
 
+//nolint:cyclop
 func TestIsTLSFilesValid(t *testing.T) {
 	testCases := []struct {
 		Name                       string
@@ -679,6 +680,7 @@ func TestIsTLSFilesValid(t *testing.T) {
 	}
 }
 
+//nolint:cyclop
 func TestIsAdminTLSFilesValid(t *testing.T) {
 	testCases := []struct {
 		Name                            string