From 4e0ab2e8ed63f0e2ed31e7ae88da7fa437e0bb26 Mon Sep 17 00:00:00 2001 From: Frederic BIDON <frederic@oneconcern.com> Date: Tue, 1 Jan 2019 15:13:16 +0100 Subject: [PATCH] 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 <frederic@oneconcern.com> --- .golangci.yml | 23 ++++++++++++++++++++ cli.go | 8 ++++--- cli_test.go | 6 ++++-- config.go | 6 +++--- cookies.go | 52 +++++++++++++++++++-------------------------- cookies_test.go | 12 +++++------ doc.go | 15 +++++++++++-- handlers.go | 24 +++++++++++---------- handlers_test.go | 4 ++-- main.go | 2 +- middleware.go | 2 +- middleware_test.go | 53 +++++++++++++++++++++++----------------------- oauth_test.go | 6 +++--- resource.go | 6 +++--- resource_test.go | 10 ++++++--- rotation.go | 2 +- rotation_test.go | 2 +- self_signed.go | 8 +++---- server.go | 14 ++++++------ server_test.go | 6 +++--- user_context.go | 6 +++--- utils.go | 27 ++++++++++++----------- utils_test.go | 8 +++---- 23 files changed, 172 insertions(+), 130 deletions(-) create mode 100644 .golangci.yml diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000..0f29bb3 --- /dev/null +++ b/.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 diff --git a/cli.go b/cli.go index 2108da7..b3c1aa7 100644 --- a/cli.go +++ b/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)) diff --git a/cli_test.go b/cli_test.go index 6a8b003..a7aade1 100644 --- a/cli_test.go +++ b/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) } diff --git a/config.go b/config.go index 4855309..cd4410a 100644 --- a/config.go +++ b/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, diff --git a/cookies.go b/cookies.go index acf4fe5..8bc82a4 100644 --- a/cookies.go +++ b/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 } diff --git a/cookies_test.go b/cookies_test.go index 0f21178..23d4e67 100644 --- a/cookies_test.go +++ b/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()) } diff --git a/doc.go b/doc.go index ca1d97d..0b11c4b 100644 --- a/doc.go +++ b/doc.go @@ -33,6 +33,8 @@ var ( version = "" ) +type contextKey int8 + const ( prog = "keycloak-gatekeeper" author = "Keycloak" @@ -40,10 +42,8 @@ const ( 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 ( diff --git a/handlers.go b/handlers.go index b8761e6..5245900 100644 --- a/handlers.go +++ b/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) } diff --git a/handlers_test.go b/handlers_test.go index 1d827fe..b4aa6c9 100644 --- a/handlers_test.go +++ b/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, }, } diff --git a/main.go b/main.go index 685fda1..f1a3aa8 100644 --- a/main.go +++ b/main.go @@ -21,5 +21,5 @@ import ( func main() { app := newOauthProxyApp() - app.Run(os.Args) + _ = app.Run(os.Args) } diff --git a/middleware.go b/middleware.go index d2fe2c6..38c4835 100644 --- a/middleware.go +++ b/middleware.go @@ -392,7 +392,7 @@ func (r *oauthProxy) identityHeadersMiddleware(custom []string) func(http.Handle } // are we filtering out the cookies if !r.config.EnableAuthorizationCookies { - filterCookies(req, cookieFilter) + _ = filterCookies(req, cookieFilter) } // inject any custom claims for claim, header := range customClaims { diff --git a/middleware_test.go b/middleware_test.go index fe78786..a51be4d 100644 --- a/middleware_test.go +++ b/middleware_test.go @@ -113,7 +113,8 @@ func (f *fakeProxy) RunTests(t *testing.T, requests []fakeRequest) { f.proxy.server.Close() }() - for i, c := range requests { + for i := range requests { + c := requests[i] var upstream fakeUpstreamResponse f.config.NoRedirects = !c.Redirects @@ -133,7 +134,7 @@ func (f *fakeProxy) RunTests(t *testing.T, requests []fakeRequest) { return nil, err } header := fmt.Sprintf("PROXY TCP4 %s 10.0.0.1 1000 2000\r\n", c.ProxyProtocol) - conn.Write([]byte(header)) + _, _ = conn.Write([]byte(header)) return conn, nil }, @@ -217,11 +218,11 @@ func (f *fakeProxy) RunTests(t *testing.T, requests []fakeRequest) { } if c.ExpectedLocation != "" { l, _ := url.Parse(resp.Header().Get("Location")) - assert.True(t, strings.Contains(l.String(), c.ExpectedLocation), "Expected location to contain %s", l.String()) + assert.True(t, strings.Contains(l.String(), c.ExpectedLocation), "expected location to contain %s", l.String()) if l.Query().Get("state") != "" { state, err := uuid.FromString(l.Query().Get("state")) if err != nil { - assert.Fail(t, "Expected state parameter with valid UUID, got: %s with error %s", state.String(), err) + assert.Fail(t, "expected state parameter with valid UUID, got: %s with error %s", state.String(), err) } } } @@ -483,6 +484,8 @@ func TestNoProxyingRequests(t *testing.T) { newFakeProxy(c).RunTests(t, requests) } +const testAdminURI = "/admin/test" + func TestStrangeAdminRequests(t *testing.T) { cfg := newFakeKeycloakConfig() cfg.Resources = []*Resource{ @@ -509,12 +512,12 @@ func TestStrangeAdminRequests(t *testing.T) { ExpectedCode: http.StatusTemporaryRedirect, }, { // check for prefix slashs - URI: "//admin/test", + URI: "/" + testAdminURI, Redirects: true, ExpectedCode: http.StatusTemporaryRedirect, }, { // check for double slashs - URI: "/admin//test", + URI: testAdminURI, Redirects: true, ExpectedCode: http.StatusTemporaryRedirect, }, @@ -525,12 +528,12 @@ func TestStrangeAdminRequests(t *testing.T) { ExpectedCode: http.StatusForbidden, }, { // check for dodgy url - URI: "//admin/../admin/test", + URI: "//admin/.." + testAdminURI, Redirects: true, ExpectedCode: http.StatusTemporaryRedirect, }, { // check for it works - URI: "//admin/test", + URI: "/" + testAdminURI, HasToken: true, Roles: []string{fakeAdminRole}, ExpectedProxy: true, @@ -1020,7 +1023,7 @@ func TestCrossSiteHandler(t *testing.T) { cfg.CorsCredentials = c.Cors.AllowCredentials cfg.CorsExposedHeaders = c.Cors.ExposedHeaders cfg.CorsHeaders = c.Cors.AllowedHeaders - cfg.CorsMaxAge = time.Duration(time.Duration(c.Cors.MaxAge) * time.Second) + cfg.CorsMaxAge = time.Duration(c.Cors.MaxAge) * time.Second cfg.CorsMethods = c.Cors.AllowedMethods cfg.CorsOrigins = c.Cors.AllowedOrigins @@ -1038,7 +1041,7 @@ func TestCheckRefreshTokens(t *testing.T) { } } p := newFakeProxy(cfg) - p.idp.setTokenExpiration(time.Duration(1000 * time.Millisecond)) + p.idp.setTokenExpiration(1000 * time.Millisecond) requests := []fakeRequest{ { @@ -1184,7 +1187,6 @@ func TestAdmissionHandlerRoles(t *testing.T) { // check to see if custom headers are hitting the upstream func TestCustomHeaders(t *testing.T) { - uri := "/admin/test" requests := []struct { Headers map[string]string Request fakeRequest @@ -1206,7 +1208,7 @@ func TestCustomHeaders(t *testing.T) { "TestHeader": "test", }, Request: fakeRequest{ - URI: uri, + URI: testAdminURI, HasToken: true, ExpectedProxy: true, ExpectedProxyHeaders: map[string]string{ @@ -1220,7 +1222,7 @@ func TestCustomHeaders(t *testing.T) { "TestHeaderTwo": "two", }, Request: fakeRequest{ - URI: uri, + URI: testAdminURI, HasToken: true, ExpectedProxy: true, ExpectedProxyHeaders: map[string]string{ @@ -1239,7 +1241,6 @@ func TestCustomHeaders(t *testing.T) { } func TestRolesAdmissionHandlerClaims(t *testing.T) { - uri := "/admin/test" requests := []struct { Matches map[string]string Request fakeRequest @@ -1248,7 +1249,7 @@ func TestRolesAdmissionHandlerClaims(t *testing.T) { { Matches: map[string]string{"cal": "test"}, Request: fakeRequest{ - URI: uri, + URI: testAdminURI, HasToken: true, ExpectedCode: http.StatusForbidden, }, @@ -1256,7 +1257,7 @@ func TestRolesAdmissionHandlerClaims(t *testing.T) { { Matches: map[string]string{"item": "^tes$"}, Request: fakeRequest{ - URI: uri, + URI: testAdminURI, HasToken: true, ExpectedCode: http.StatusForbidden, }, @@ -1264,7 +1265,7 @@ func TestRolesAdmissionHandlerClaims(t *testing.T) { { Matches: map[string]string{"item": "^tes$"}, Request: fakeRequest{ - URI: uri, + URI: testAdminURI, HasToken: true, TokenClaims: jose.Claims{"item": "tes"}, ExpectedProxy: true, @@ -1274,7 +1275,7 @@ func TestRolesAdmissionHandlerClaims(t *testing.T) { { Matches: map[string]string{"item": "not_match"}, Request: fakeRequest{ - URI: uri, + URI: testAdminURI, HasToken: true, TokenClaims: jose.Claims{"item": "test"}, ExpectedCode: http.StatusForbidden, @@ -1283,7 +1284,7 @@ func TestRolesAdmissionHandlerClaims(t *testing.T) { { Matches: map[string]string{"item": "^test", "found": "something"}, Request: fakeRequest{ - URI: uri, + URI: testAdminURI, HasToken: true, TokenClaims: jose.Claims{"item": "test"}, ExpectedCode: http.StatusForbidden, @@ -1292,7 +1293,7 @@ func TestRolesAdmissionHandlerClaims(t *testing.T) { { Matches: map[string]string{"item": "^test", "found": "something"}, Request: fakeRequest{ - URI: uri, + URI: testAdminURI, HasToken: true, TokenClaims: jose.Claims{ "item": "tester", @@ -1305,7 +1306,7 @@ func TestRolesAdmissionHandlerClaims(t *testing.T) { { Matches: map[string]string{"item": ".*"}, Request: fakeRequest{ - URI: uri, + URI: testAdminURI, HasToken: true, TokenClaims: jose.Claims{"item": "test"}, ExpectedProxy: true, @@ -1315,7 +1316,7 @@ func TestRolesAdmissionHandlerClaims(t *testing.T) { { Matches: map[string]string{"item": "^t.*$"}, Request: fakeRequest{ - URI: uri, + URI: testAdminURI, HasToken: true, TokenClaims: jose.Claims{"item": "test"}, ExpectedProxy: true, @@ -1326,7 +1327,7 @@ func TestRolesAdmissionHandlerClaims(t *testing.T) { { Matches: map[string]string{"item": "^t.*t"}, Request: fakeRequest{ - URI: uri, + URI: testAdminURI, HasToken: true, TokenClaims: jose.Claims{"item": []string{"nonMatchingClaim", "test", "anotherNonMatching"}}, ExpectedProxy: true, @@ -1336,7 +1337,7 @@ func TestRolesAdmissionHandlerClaims(t *testing.T) { { Matches: map[string]string{"item": "^t.*t"}, Request: fakeRequest{ - URI: uri, + URI: testAdminURI, HasToken: true, TokenClaims: jose.Claims{"item": []string{"1test", "2test", "3test"}}, ExpectedProxy: false, @@ -1346,7 +1347,7 @@ func TestRolesAdmissionHandlerClaims(t *testing.T) { { Matches: map[string]string{"item": "^t.*t"}, Request: fakeRequest{ - URI: uri, + URI: testAdminURI, HasToken: true, TokenClaims: jose.Claims{"item": []string{}}, ExpectedProxy: false, @@ -1359,7 +1360,7 @@ func TestRolesAdmissionHandlerClaims(t *testing.T) { "item2": "^another", }, Request: fakeRequest{ - URI: uri, + URI: testAdminURI, HasToken: true, TokenClaims: jose.Claims{ "item1": []string{"randomItem", "test"}, diff --git a/oauth_test.go b/oauth_test.go index 21029c1..70786be 100644 --- a/oauth_test.go +++ b/oauth_test.go @@ -259,7 +259,7 @@ func (r *fakeAuthServer) tokenHandler(w http.ResponseWriter, req *http.Request) } renderJSON(http.StatusUnauthorized, w, req, map[string]string{ "error": "invalid_grant", - "error_description": "Invalid user credentials", + "error_description": "invalid user credentials", }) case oauth2.GrantTypeRefreshToken: fallthrough @@ -292,11 +292,11 @@ func TestTokenExpired(t *testing.T) { OK bool }{ { - Expire: time.Duration(1 * time.Hour), + Expire: 1 * time.Hour, OK: true, }, { - Expire: time.Duration(-5 * time.Hour), + Expire: -5 * time.Hour, }, } for i, x := range cs { diff --git a/resource.go b/resource.go index 2b1b1d6..f02ef8c 100644 --- a/resource.go +++ b/resource.go @@ -47,7 +47,7 @@ func (r *Resource) parse(resource string) (*Resource, error) { case "methods": r.Methods = strings.Split(kp[1], ",") if len(r.Methods) == 1 { - if r.Methods[0] == "any" || r.Methods[0] == "ANY" { + if strings.EqualFold(r.Methods[0], anyMethod) { r.Methods = allHTTPMethods } } @@ -91,7 +91,7 @@ func (r *Resource) valid() error { } // step: add any of no methods - if len(r.Methods) <= 0 { + if len(r.Methods) == 0 { r.Methods = allHTTPMethods } // step: check the method is valid @@ -116,7 +116,7 @@ func (r Resource) String() string { } roles := "authentication only" - methods := "ANY" + methods := anyMethod if len(r.Roles) > 0 { roles = strings.Join(r.Roles, ",") diff --git a/resource_test.go b/resource_test.go index 71f2401..26cc9c0 100644 --- a/resource_test.go +++ b/resource_test.go @@ -133,9 +133,13 @@ func TestIsValid(t *testing.T) { } } +var expectedRoles = []string{"1", "2", "3"} + +const rolesList = "1,2,3" + func TestResourceString(t *testing.T) { resource := &Resource{ - Roles: []string{"1", "2", "3"}, + Roles: expectedRoles, } if s := resource.String(); s == "" { t.Error("we should have received a string") @@ -144,10 +148,10 @@ func TestResourceString(t *testing.T) { func TestGetRoles(t *testing.T) { resource := &Resource{ - Roles: []string{"1", "2", "3"}, + Roles: expectedRoles, } - if resource.getRoles() != "1,2,3" { + if resource.getRoles() != rolesList { t.Error("the resource roles not as expected") } } diff --git a/rotation.go b/rotation.go index 91a2c7a..96b7a61 100644 --- a/rotation.go +++ b/rotation.go @@ -92,7 +92,7 @@ func (c *certificationRotation) watch() error { // @metric inform of the rotation certificateRotationMetric.Inc() // step: load the new certificate - c.storeCertificate(certificate) + _ = c.storeCertificate(certificate) // step: print a debug message for us c.log.Info("replacing the server certifacte with updated version") } diff --git a/rotation_test.go b/rotation_test.go index 01f96af..2c99bbb 100644 --- a/rotation_test.go +++ b/rotation_test.go @@ -63,7 +63,7 @@ func TestGetCertificate(t *testing.T) { func TestLoadCertificate(t *testing.T) { c := newTestCertificateRotator(t) assert.NotEmpty(t, c.certificate) - c.storeCertificate(tls.Certificate{}) + _ = c.storeCertificate(tls.Certificate{}) crt, err := c.GetCertificate(nil) assert.NoError(t, err) assert.Equal(t, &tls.Certificate{}, crt) diff --git a/self_signed.go b/self_signed.go index 6bc4097..64bf317 100644 --- a/self_signed.go +++ b/self_signed.go @@ -46,7 +46,7 @@ type selfSignedCertificate struct { // newSelfSignedCertificate creates and returns a self signed certificate manager func newSelfSignedCertificate(hostnames []string, expiry time.Duration, log *zap.Logger) (*selfSignedCertificate, error) { - if len(hostnames) <= 0 { + if len(hostnames) == 0 { return nil, errors.New("no hostnames specified") } if expiry < 5*time.Minute { @@ -94,17 +94,17 @@ func (c *selfSignedCertificate) rotate(ctx context.Context) error { for { expires := time.Now().Add(c.expiration).Add(-5 * time.Minute) - ticker := expires.Sub(time.Now()) + ticker := time.Until(expires) select { case <-ctx.Done(): return case <-time.After(ticker): } - c.log.Info("going to sleep until required for rotation", zap.Time("expires", expires), zap.Duration("duration", expires.Sub(time.Now()))) + c.log.Info("going to sleep until required for rotation", zap.Time("expires", expires), zap.Duration("duration", time.Until(expires))) // @step: got to sleep until we need to rotate - time.Sleep(expires.Sub(time.Now())) + time.Sleep(time.Until(expires)) // @step: create a new certificate for us cert, _ := createCertificate(c.privateKey, c.hostnames, c.expiration) diff --git a/server.go b/server.go index 0289958..becbbfd 100644 --- a/server.go +++ b/server.go @@ -63,7 +63,7 @@ type oauthProxy struct { } func init() { - time.LoadLocation("UTC") // ensure all time is in UTC + _, _ = time.LoadLocation("UTC") // ensure all time is in UTC [NOTE(fredbi): no this does just nothing] runtime.GOMAXPROCS(runtime.NumCPU()) // set the core prometheus.MustRegister(certificateRotationMetric) prometheus.MustRegister(latencyMetric) @@ -466,7 +466,7 @@ func (r *oauthProxy) createHTTPListener(config listenerConfig) (net.Listener, er // @check if the socket requires TLS if config.useSelfSignedTLS || config.useLetsEncryptTLS || config.useFileTLS { getCertificate := func(hello *tls.ClientHelloInfo) (*tls.Certificate, error) { - return nil, errors.New("Not configured") + return nil, errors.New("not configured") } if config.useLetsEncryptTLS { @@ -567,9 +567,10 @@ func (r *oauthProxy) createUpstreamProxy(upstream *url.URL) error { } upstream.Path = "" upstream.Host = "domain-sock" - upstream.Scheme = "http" + upstream.Scheme = unsecureScheme } // create the upstream tls configure + //nolint:gas tlsConfig := &tls.Config{InsecureSkipVerify: r.config.SkipUpstreamTLSVerify} // are we using a client certificate @@ -659,9 +660,9 @@ func (r *oauthProxy) newOpenIDClient() (*oidc.Client, oidc.ProviderConfig, *http Transport: &http.Transport{ Proxy: func(_ *http.Request) (*url.URL, error) { if r.config.OpenIDProviderProxy != "" { - idpProxyURL, err := url.Parse(r.config.OpenIDProviderProxy) - if err != nil { - r.log.Warn("invalid proxy address for open IDP provider proxy", zap.Error(err)) + idpProxyURL, erp := url.Parse(r.config.OpenIDProviderProxy) + if erp != nil { + r.log.Warn("invalid proxy address for open IDP provider proxy", zap.Error(erp)) return nil, nil } return idpProxyURL, nil @@ -670,6 +671,7 @@ func (r *oauthProxy) newOpenIDClient() (*oidc.Client, oidc.ProviderConfig, *http return nil, nil }, TLSClientConfig: &tls.Config{ + //nolint:gas InsecureSkipVerify: r.config.SkipOpenIDProviderTLSVerify, }, }, diff --git a/server_test.go b/server_test.go index e616852..0fd25d6 100644 --- a/server_test.go +++ b/server_test.go @@ -108,8 +108,8 @@ func TestForwardingProxy(t *testing.T) { cfg := newFakeKeycloakConfig() cfg.EnableForwarding = true cfg.ForwardingDomains = []string{} - cfg.ForwardingUsername = "test" - cfg.ForwardingPassword = "test" + cfg.ForwardingUsername = validUsername + cfg.ForwardingPassword = validPassword s := httptest.NewServer(&fakeUpstreamService{}) requests := []fakeRequest{ { @@ -537,7 +537,7 @@ func (f *fakeUpstreamService) ServeHTTP(w http.ResponseWriter, r *http.Request) Address: r.RemoteAddr, Headers: r.Header, }) - w.Write(content) + _, _ = w.Write(content) } type fakeToken struct { diff --git a/user_context.go b/user_context.go index 7f2e01c..80227e0 100644 --- a/user_context.go +++ b/user_context.go @@ -46,11 +46,11 @@ func extractIdentity(token jose.JWT) (*userContext, error) { if err == nil && found { audiences = append(audiences, aud) } else { - if aud, found, err := claims.StringsClaim(claimAudience); err != nil || !found { + aud, found, erc := claims.StringsClaim(claimAudience) + if erc != nil || !found { return nil, ErrNoTokenAudience - } else { - audiences = aud } + audiences = aud } // @step: extract the realm roles diff --git a/utils.go b/utils.go index f22ea7f..f90147d 100644 --- a/utils.go +++ b/utils.go @@ -19,6 +19,7 @@ import ( "crypto/aes" "crypto/cipher" "crypto/rand" + cryptorand "crypto/rand" "crypto/rsa" sha "crypto/sha256" "crypto/tls" @@ -46,7 +47,7 @@ import ( "github.com/coreos/go-oidc/jose" "github.com/urfave/cli" - "gopkg.in/yaml.v2" + yaml "gopkg.in/yaml.v2" ) var ( @@ -69,7 +70,7 @@ var ( // createCertificate is responsible for creating a certificate func createCertificate(key *rsa.PrivateKey, hostnames []string, expire time.Duration) (tls.Certificate, error) { // @step: create a serial for the certificate - serial, err := rand.Int(rand.Reader, new(big.Int).Lsh(big.NewInt(1), 128)) + serial, err := cryptorand.Int(rand.Reader, new(big.Int).Lsh(big.NewInt(1), 128)) if err != nil { return tls.Certificate{}, err } @@ -102,7 +103,7 @@ func createCertificate(key *rsa.PrivateKey, hostnames []string, expire time.Dura } // @step: create the certificate - cert, err := x509.CreateCertificate(rand.Reader, &template, &template, &key.PublicKey, key) + cert, err := x509.CreateCertificate(cryptorand.Reader, &template, &template, &key.PublicKey, key) if err != nil { return tls.Certificate{}, err } @@ -155,7 +156,7 @@ func encryptDataBlock(plaintext, key []byte) ([]byte, error) { return []byte{}, err } nonce := make([]byte, gcm.NonceSize()) - if _, err = io.ReadFull(rand.Reader, nonce); err != nil { + if _, err = io.ReadFull(cryptorand.Reader, nonce); err != nil { return nil, err } @@ -299,14 +300,15 @@ func containsSubString(value string, list []string) bool { return false } -// tryDialEndpoint dials the upstream endpoint via plain +// tryDialEndpoint dials the upstream endpoint via plain HTTP func tryDialEndpoint(location *url.URL) (net.Conn, error) { switch dialAddress := dialAddress(location); location.Scheme { - case httpSchema: + case unsecureScheme: return net.Dial("tcp", dialAddress) default: return tls.Dial("tcp", dialAddress, &tls.Config{ - Rand: rand.Reader, + Rand: cryptorand.Reader, + //nolint:gas InsecureSkipVerify: true, }) } @@ -352,8 +354,8 @@ func tryUpdateConnection(req *http.Request, writer http.ResponseWriter, endpoint // @step: copy the data between client and upstream endpoint var wg sync.WaitGroup wg.Add(2) - go transferBytes(server, client, &wg) - go transferBytes(client, server, &wg) + go func() { _, _ = transferBytes(server, client, &wg) }() + go func() { _, _ = transferBytes(client, server, &wg) }() wg.Wait() return nil @@ -364,7 +366,7 @@ func dialAddress(location *url.URL) string { items := strings.Split(location.Host, ":") if len(items) != 2 { switch location.Scheme { - case httpSchema: + case unsecureScheme: return location.Host + ":80" default: return location.Host + ":443" @@ -387,10 +389,11 @@ func findCookie(name string, cookies []*http.Cookie) *http.Cookie { // toHeader is a helper method to play nice in the headers func toHeader(v string) string { - var list []string + symbols := symbolsFilter.Split(v, -1) + list := make([]string, 0, len(symbols)) // step: filter out any symbols and convert to dashes - for _, x := range symbolsFilter.Split(v, -1) { + for _, x := range symbols { list = append(list, capitalize(x)) } diff --git a/utils_test.go b/utils_test.go index a55917f..70aac39 100644 --- a/utils_test.go +++ b/utils_test.go @@ -104,7 +104,7 @@ func TestGetRequestHostURL(t *testing.T) { TLS: c.TLS, } if c.HostHeader != "" { - request.Header = make(http.Header, 0) + request.Header = make(http.Header) request.Header.Set("X-Forwarded-Host", c.HostHeader) } assert.Equal(t, c.Expected, getRequestHostURL(request), "case %d, expected: %s, got: %s", i, c.Expected, getRequestHostURL(request)) @@ -114,7 +114,7 @@ func TestGetRequestHostURL(t *testing.T) { func BenchmarkUUID(b *testing.B) { for n := 0; n < b.N; n++ { s := uuid.NewV1() - s.String() + _ = s.String() } } @@ -200,7 +200,7 @@ func TestEncryptedText(t *testing.T) { func BenchmarkEncryptDataBlock(b *testing.B) { for n := 0; n < b.N; n++ { - encryptDataBlock(fakePlainText, fakeKey) + _, _ = encryptDataBlock(fakePlainText, fakeKey) } } @@ -208,7 +208,7 @@ func BenchmarkEncodeText(b *testing.B) { text := string(fakePlainText) key := string(fakeKey) for n := 0; n < b.N; n++ { - encodeText(text, key) + _, _ = encodeText(text, key) } } -- GitLab