From dd05e7ff0bad417b088c9e67e5eaa624e30e3ddc Mon Sep 17 00:00:00 2001 From: Mitsuo Heijo Date: Tue, 14 Apr 2020 17:36:44 +0900 Subject: [PATCH] Add new linters (#486) * add new linters and fix issues * fix deprecated warnings * simplify return * update CHANGELOG * fix staticcheck issues * remove a deprecated linter, minor fixes of variable initialization --- .golangci.yml | 24 +++++++++++++++++++++- CHANGELOG.md | 1 + oauthproxy.go | 16 ++++++--------- oauthproxy_test.go | 30 ++++++++++++---------------- options.go | 8 ++++---- options_test.go | 12 +++++------ pkg/logger/logger.go | 10 +++++----- pkg/requests/requests_test.go | 4 ++-- pkg/sessions/cookie/session_store.go | 9 +++------ pkg/sessions/redis/redis_store.go | 6 +++--- providers/bitbucket.go | 4 ++-- providers/github.go | 2 +- providers/gitlab.go | 6 +----- providers/google.go | 8 +++++--- providers/keycloak.go | 2 +- providers/logingov.go | 5 ++--- providers/oidc.go | 6 +----- providers/oidc_test.go | 7 ++++++- providers/provider_data.go | 2 +- providers/provider_default.go | 3 +-- watcher.go | 2 +- 21 files changed, 88 insertions(+), 79 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index a0658e1c..9ee860a1 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -9,5 +9,27 @@ linters: - deadcode - gofmt - goimports - enable-all: false + - gosimple + - staticcheck + - structcheck + - typecheck + - unused + - varcheck + - bodyclose + - dogsled + - goprintffuncname + - misspell + - prealloc + - scopelint + - stylecheck + - unconvert + - gocritic disable-all: true +issues: + exclude-rules: + - path: _test\.go + linters: + - scopelint + - bodyclose + - unconvert + - gocritic diff --git a/CHANGELOG.md b/CHANGELOG.md index 2a4bf5ed..a6eaa1fb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ ## Changes since v5.1.0 +- [#486](https://github.com/oauth2-proxy/oauth2-proxy/pull/486) Add new linters (@johejo) - [#440](https://github.com/oauth2-proxy/oauth2-proxy/pull/440) Switch Azure AD Graph API to Microsoft Graph API (@johejo) - [#453](https://github.com/oauth2-proxy/oauth2-proxy/pull/453) Prevent browser caching during auth flow (@johejo) - [#481](https://github.com/oauth2-proxy/oauth2-proxy/pull/481) Update Okta docs (@trevorbox) diff --git a/oauthproxy.go b/oauthproxy.go index 1d49b597..bdf1f6a0 100644 --- a/oauthproxy.go +++ b/oauthproxy.go @@ -324,8 +324,7 @@ func (p *OAuthProxy) GetRedirectURI(host string) string { if p.redirectURL.Host != "" { return p.redirectURL.String() } - var u url.URL - u = *p.redirectURL + u := *p.redirectURL if u.Scheme == "" { if p.CookieSecure { u.Scheme = httpsScheme @@ -695,7 +694,7 @@ func (p *OAuthProxy) SignIn(rw http.ResponseWriter, req *http.Request) { if ok { session := &sessionsapi.SessionState{User: user} p.SaveSession(rw, req, session) - http.Redirect(rw, req, redirect, 302) + http.Redirect(rw, req, redirect, http.StatusFound) } else { if p.SkipProviderButton { p.OAuthStart(rw, req) @@ -734,7 +733,7 @@ func (p *OAuthProxy) SignOut(rw http.ResponseWriter, req *http.Request) { return } p.ClearSessionCookie(rw, req) - http.Redirect(rw, req, redirect, 302) + http.Redirect(rw, req, redirect, http.StatusFound) } // OAuthStart starts the OAuth2 authentication flow @@ -754,7 +753,7 @@ func (p *OAuthProxy) OAuthStart(rw http.ResponseWriter, req *http.Request) { return } redirectURI := p.GetRedirectURI(req.Host) - http.Redirect(rw, req, p.provider.GetLoginURL(redirectURI, fmt.Sprintf("%v:%v", nonce, redirect)), 302) + http.Redirect(rw, req, p.provider.GetLoginURL(redirectURI, fmt.Sprintf("%v:%v", nonce, redirect)), http.StatusFound) } // OAuthCallback is the OAuth2 authentication flow callback that finishes the @@ -817,7 +816,7 @@ func (p *OAuthProxy) OAuthCallback(rw http.ResponseWriter, req *http.Request) { p.ErrorPage(rw, 500, "Internal Error", "Internal Error") return } - http.Redirect(rw, req, redirect, 302) + http.Redirect(rw, req, redirect, http.StatusFound) } else { logger.PrintAuthf(session.Email, req, logger.AuthFailure, "Invalid authentication via OAuth2: unauthorized") p.ErrorPage(rw, 403, "Permission Denied", "Invalid Account") @@ -1096,10 +1095,7 @@ func (p *OAuthProxy) CheckBasicAuth(req *http.Request) (*sessionsapi.SessionStat // isAjax checks if a request is an ajax request func isAjax(req *http.Request) bool { - acceptValues, ok := req.Header["accept"] - if !ok { - acceptValues = req.Header["Accept"] - } + acceptValues := req.Header.Values("Accept") const ajaxReq = applicationJSON for _, v := range acceptValues { if v == ajaxReq { diff --git a/oauthproxy_test.go b/oauthproxy_test.go index 87668f10..47b27743 100644 --- a/oauthproxy_test.go +++ b/oauthproxy_test.go @@ -64,7 +64,6 @@ func TestWebSocketProxy(t *testing.T) { if err != nil { t.Fatalf("err %s", err) } - return }), } backend := httptest.NewServer(&handler) @@ -426,7 +425,7 @@ func TestBasicAuthPassword(t *testing.T) { if rw.Code >= 400 { t.Fatalf("expected 3xx got %d", rw.Code) } - cookie := rw.HeaderMap["Set-Cookie"][1] + cookie := rw.Header().Values("Set-Cookie")[1] cookieName := proxy.CookieName var value string @@ -614,7 +613,7 @@ func (patTest *PassAccessTokenTest) getCallbackEndpoint() (httpCode int, } req.AddCookie(patTest.proxy.MakeCSRFCookie(req, "nonce", time.Hour, time.Now())) patTest.proxy.ServeHTTP(rw, req) - return rw.Code, rw.HeaderMap["Set-Cookie"][1] + return rw.Code, rw.Header().Values("Set-Cookie")[1] } // getEndpointWithCookie makes a requests againt the oauthproxy with passed requestPath @@ -691,7 +690,7 @@ func TestStaticProxyUpstream(t *testing.T) { } assert.NotEqual(t, nil, cookie) - // Now we make a regular request againts the upstream proxy; And validate + // Now we make a regular request against the upstream proxy; And validate // the returned status code through the static proxy. code, payload := patTest.getEndpointWithCookie(cookie, "/static-proxy") if code != 200 { @@ -824,8 +823,6 @@ type ProcessCookieTest struct { proxy *OAuthProxy rw *httptest.ResponseRecorder req *http.Request - provider TestProvider - responseCode int validateUser bool } @@ -910,7 +907,7 @@ func TestProcessCookieNoCookieError(t *testing.T) { pcTest := NewProcessCookieTestWithDefaults() session, err := pcTest.LoadCookiedSession() - assert.Equal(t, "Cookie \"_oauth2_proxy\" not present", err.Error()) + assert.Equal(t, "cookie \"_oauth2_proxy\" not present", err.Error()) if session != nil { t.Errorf("expected nil session. got %#v", session) } @@ -1072,8 +1069,8 @@ func TestAuthOnlyEndpointSetXAuthRequestHeaders(t *testing.T) { pcTest.proxy.ServeHTTP(pcTest.rw, pcTest.req) assert.Equal(t, http.StatusAccepted, pcTest.rw.Code) - assert.Equal(t, "oauth_user", pcTest.rw.HeaderMap["X-Auth-Request-User"][0]) - assert.Equal(t, "oauth_user@example.com", pcTest.rw.HeaderMap["X-Auth-Request-Email"][0]) + assert.Equal(t, "oauth_user", pcTest.rw.Header().Get("X-Auth-Request-User")) + assert.Equal(t, "oauth_user@example.com", pcTest.rw.Header().Get("X-Auth-Request-Email")) } func TestAuthOnlyEndpointSetBasicAuthTrueRequestHeaders(t *testing.T) { @@ -1103,10 +1100,10 @@ func TestAuthOnlyEndpointSetBasicAuthTrueRequestHeaders(t *testing.T) { pcTest.proxy.ServeHTTP(pcTest.rw, pcTest.req) assert.Equal(t, http.StatusAccepted, pcTest.rw.Code) - assert.Equal(t, "oauth_user", pcTest.rw.HeaderMap["X-Auth-Request-User"][0]) - assert.Equal(t, "oauth_user@example.com", pcTest.rw.HeaderMap["X-Auth-Request-Email"][0]) + assert.Equal(t, "oauth_user", pcTest.rw.Header().Values("X-Auth-Request-User")[0]) + assert.Equal(t, "oauth_user@example.com", pcTest.rw.Header().Values("X-Auth-Request-Email")[0]) expectedHeader := "Basic " + base64.StdEncoding.EncodeToString([]byte("oauth_user:"+pcTest.opts.BasicAuthPassword)) - assert.Equal(t, expectedHeader, pcTest.rw.HeaderMap["Authorization"][0]) + assert.Equal(t, expectedHeader, pcTest.rw.Header().Values("Authorization")[0]) } func TestAuthOnlyEndpointSetBasicAuthFalseRequestHeaders(t *testing.T) { @@ -1136,9 +1133,9 @@ func TestAuthOnlyEndpointSetBasicAuthFalseRequestHeaders(t *testing.T) { pcTest.proxy.ServeHTTP(pcTest.rw, pcTest.req) assert.Equal(t, http.StatusAccepted, pcTest.rw.Code) - assert.Equal(t, "oauth_user", pcTest.rw.HeaderMap["X-Auth-Request-User"][0]) - assert.Equal(t, "oauth_user@example.com", pcTest.rw.HeaderMap["X-Auth-Request-Email"][0]) - assert.Equal(t, 0, len(pcTest.rw.HeaderMap["Authorization"]), "should not have Authorization header entries") + assert.Equal(t, "oauth_user", pcTest.rw.Header().Values("X-Auth-Request-User")[0]) + assert.Equal(t, "oauth_user@example.com", pcTest.rw.Header().Values("X-Auth-Request-Email")[0]) + assert.Equal(t, 0, len(pcTest.rw.Header().Values("Authorization")), "should not have Authorization header entries") } func TestAuthSkippedForPreflightRequests(t *testing.T) { @@ -1462,8 +1459,7 @@ type NoOpKeySet struct { func (NoOpKeySet) VerifySignature(ctx context.Context, jwt string) (payload []byte, err error) { splitStrings := strings.Split(jwt, ".") payloadString := splitStrings[1] - jsonString, err := base64.RawURLEncoding.DecodeString(payloadString) - return []byte(jsonString), err + return base64.RawURLEncoding.DecodeString(payloadString) } func TestGetJwtSession(t *testing.T) { diff --git a/options.go b/options.go index 1a517b41..f9abe3c5 100644 --- a/options.go +++ b/options.go @@ -290,7 +290,7 @@ func (o *Options) Validate() error { } } - if o.PreferEmailToUser == true && o.PassBasicAuth == false && o.PassUserHeaders == false { + if o.PreferEmailToUser && !o.PassBasicAuth && !o.PassUserHeaders { msgs = append(msgs, "PreferEmailToUser should only be used with PassBasicAuth or PassUserHeaders") } @@ -349,7 +349,7 @@ func (o *Options) Validate() error { if string(secretBytes(o.CookieSecret)) != o.CookieSecret { decoded = true } - if validCookieSecretSize == false { + if !validCookieSecretSize { var suffix string if decoded { suffix = fmt.Sprintf(" note: cookie secret was base64 decoded from %q", o.CookieSecret) @@ -414,7 +414,7 @@ func (o *Options) Validate() error { msgs = setupLogger(o, msgs) if len(msgs) != 0 { - return fmt.Errorf("Invalid configuration:\n %s", + return fmt.Errorf("invalid configuration:\n %s", strings.Join(msgs, "\n ")) } return nil @@ -545,7 +545,7 @@ func parseSignatureKey(o *Options, msgs []string) []string { // parseJwtIssuers takes in an array of strings in the form of issuer=audience // and parses to an array of jwtIssuer structs. func parseJwtIssuers(issuers []string, msgs []string) ([]jwtIssuer, []string) { - var parsedIssuers []jwtIssuer + parsedIssuers := make([]jwtIssuer, 0, len(issuers)) for _, jwtVerifier := range issuers { components := strings.Split(jwtVerifier, "=") if len(components) < 2 { diff --git a/options_test.go b/options_test.go index 7dcd0934..a6bbc9ce 100644 --- a/options_test.go +++ b/options_test.go @@ -31,7 +31,7 @@ func testOptions() *Options { func errorMsg(msgs []string) string { result := make([]string, 0) - result = append(result, "Invalid configuration:") + result = append(result, "invalid configuration:") result = append(result, msgs...) return strings.Join(result, "\n ") } @@ -278,7 +278,7 @@ func TestValidateSignatureKeyInvalidSpec(t *testing.T) { o := testOptions() o.SignatureKey = "invalid spec" err := o.Validate() - assert.Equal(t, err.Error(), "Invalid configuration:\n"+ + assert.Equal(t, err.Error(), "invalid configuration:\n"+ " invalid signature hash:key spec: "+o.SignatureKey) } @@ -286,7 +286,7 @@ func TestValidateSignatureKeyUnsupportedAlgorithm(t *testing.T) { o := testOptions() o.SignatureKey = "unsupported:default secret" err := o.Validate() - assert.Equal(t, err.Error(), "Invalid configuration:\n"+ + assert.Equal(t, err.Error(), "invalid configuration:\n"+ " unsupported signature hash algorithm: "+o.SignatureKey) } @@ -300,7 +300,7 @@ func TestValidateCookieBadName(t *testing.T) { o := testOptions() o.CookieName = "_bad_cookie_name{}" err := o.Validate() - assert.Equal(t, err.Error(), "Invalid configuration:\n"+ + assert.Equal(t, err.Error(), "invalid configuration:\n"+ fmt.Sprintf(" invalid cookie name: %q", o.CookieName)) } @@ -311,8 +311,8 @@ func TestSkipOIDCDiscovery(t *testing.T) { o.SkipOIDCDiscovery = true err := o.Validate() - assert.Equal(t, "Invalid configuration:\n"+ - fmt.Sprintf(" missing setting: login-url\n missing setting: redeem-url\n missing setting: oidc-jwks-url"), err.Error()) + assert.Equal(t, "invalid configuration:\n"+ + " missing setting: login-url\n missing setting: redeem-url\n missing setting: oidc-jwks-url", err.Error()) o.LoginURL = "https://login.microsoftonline.com/fabrikamb2c.onmicrosoft.com/oauth2/v2.0/authorize?p=b2c_1_sign_in" o.RedeemURL = "https://login.microsoftonline.com/fabrikamb2c.onmicrosoft.com/oauth2/v2.0/token?p=b2c_1_sign_in" diff --git a/pkg/logger/logger.go b/pkg/logger/logger.go index 962a2c93..0664f843 100644 --- a/pkg/logger/logger.go +++ b/pkg/logger/logger.go @@ -139,10 +139,10 @@ func (l *Logger) Output(calldepth int, message string) { l.writer.Write([]byte("\n")) } -// PrintAuth writes auth info to the logger. Requires an http.Request to +// PrintAuthf writes auth info to the logger. Requires an http.Request to // log request details. Remaining arguments are handled in the manner of // fmt.Sprintf. Writes a final newline to the end of every message. -func (l *Logger) PrintAuth(username string, req *http.Request, status AuthStatus, format string, a ...interface{}) { +func (l *Logger) PrintAuthf(username string, req *http.Request, status AuthStatus, format string, a ...interface{}) { if !l.authEnabled { return } @@ -166,7 +166,7 @@ func (l *Logger) PrintAuth(username string, req *http.Request, status AuthStatus Timestamp: FormatTimestamp(now), UserAgent: fmt.Sprintf("%q", req.UserAgent()), Username: username, - Status: fmt.Sprintf("%s", status), + Status: string(status), Message: fmt.Sprintf(format, a...), }) @@ -185,7 +185,7 @@ func (l *Logger) PrintReq(username, upstream string, req *http.Request, url url. return } - duration := float64(time.Now().Sub(ts)) / float64(time.Second) + duration := float64(time.Since(ts)) / float64(time.Second) if username == "" { username = "-" @@ -481,7 +481,7 @@ func Panicln(v ...interface{}) { // PrintAuthf writes authentication details to the standard logger. // Arguments are handled in the manner of fmt.Printf. func PrintAuthf(username string, req *http.Request, status AuthStatus, format string, a ...interface{}) { - std.PrintAuth(username, req, status, format, a...) + std.PrintAuthf(username, req, status, format, a...) } // PrintReq writes request details to the standard logger. diff --git a/pkg/requests/requests_test.go b/pkg/requests/requests_test.go index c9ec4e88..acd9b0b8 100644 --- a/pkg/requests/requests_test.go +++ b/pkg/requests/requests_test.go @@ -88,9 +88,9 @@ func TestRequestUnparsedResponseUsingAccessTokenParameter(t *testing.T) { response, err := RequestUnparsedResponse( backend.URL+"?access_token=my_token", nil) + assert.Equal(t, nil, err) defer response.Body.Close() - assert.Equal(t, nil, err) assert.Equal(t, 200, response.StatusCode) body, err := ioutil.ReadAll(response.Body) assert.Equal(t, nil, err) @@ -124,9 +124,9 @@ func TestRequestUnparsedResponseUsingHeaders(t *testing.T) { headers := make(http.Header) headers.Set("Auth", "my_token") response, err := RequestUnparsedResponse(backend.URL, headers) + assert.Equal(t, nil, err) defer response.Body.Close() - assert.Equal(t, nil, err) assert.Equal(t, 200, response.StatusCode) body, err := ioutil.ReadAll(response.Body) assert.Equal(t, nil, err) diff --git a/pkg/sessions/cookie/session_store.go b/pkg/sessions/cookie/session_store.go index 727f915b..1ef0134e 100644 --- a/pkg/sessions/cookie/session_store.go +++ b/pkg/sessions/cookie/session_store.go @@ -52,11 +52,11 @@ func (s *SessionStore) Load(req *http.Request) (*sessions.SessionState, error) { c, err := loadCookie(req, s.CookieOptions.CookieName) if err != nil { // always http.ErrNoCookie - return nil, fmt.Errorf("Cookie %q not present", s.CookieOptions.CookieName) + return nil, fmt.Errorf("cookie %q not present", s.CookieOptions.CookieName) } val, _, ok := encryption.Validate(c, s.CookieOptions.CookieSecret, s.CookieOptions.CookieExpire) if !ok { - return nil, errors.New("Cookie Signature not valid") + return nil, errors.New("cookie signature not valid") } session, err := utils.SessionFromCookie(val, s.CookieCipher) @@ -69,8 +69,6 @@ func (s *SessionStore) Load(req *http.Request) (*sessions.SessionState, error) { // Clear clears any saved session information by writing a cookie to // clear the session func (s *SessionStore) Clear(rw http.ResponseWriter, req *http.Request) error { - var cookies []*http.Cookie - // matches CookieName, CookieName_ var cookieNameRegex = regexp.MustCompile(fmt.Sprintf("^%s(_\\d+)?$", s.CookieOptions.CookieName)) @@ -79,7 +77,6 @@ func (s *SessionStore) Clear(rw http.ResponseWriter, req *http.Request) error { clearCookie := s.makeCookie(req, c.Name, "", time.Hour*-1, time.Now()) http.SetCookie(rw, clearCookie) - cookies = append(cookies, clearCookie) } } @@ -174,7 +171,7 @@ func loadCookie(req *http.Request, cookieName string) (*http.Cookie, error) { } } if len(cookies) == 0 { - return nil, fmt.Errorf("Could not find cookie %s", cookieName) + return nil, fmt.Errorf("could not find cookie %s", cookieName) } return joinCookies(cookies) } diff --git a/pkg/sessions/redis/redis_store.go b/pkg/sessions/redis/redis_store.go index 3f9271e1..4d6ce9cf 100644 --- a/pkg/sessions/redis/redis_store.go +++ b/pkg/sessions/redis/redis_store.go @@ -79,7 +79,7 @@ func newRedisCmdable(opts options.RedisStoreOptions) (Client, error) { return nil, fmt.Errorf("unable to parse redis url: %s", err) } - if opts.RedisInsecureTLS != false { + if opts.RedisInsecureTLS { opt.TLSConfig.InsecureSkipVerify = true } @@ -149,7 +149,7 @@ func (store *SessionStore) Load(req *http.Request) (*sessions.SessionState, erro val, _, ok := encryption.Validate(requestCookie, store.CookieOptions.CookieSecret, store.CookieOptions.CookieExpire) if !ok { - return nil, fmt.Errorf("Cookie Signature not valid") + return nil, fmt.Errorf("cookie signature not valid") } ctx := req.Context() session, err := store.loadSessionFromString(ctx, val) @@ -209,7 +209,7 @@ func (store *SessionStore) Clear(rw http.ResponseWriter, req *http.Request) erro val, _, ok := encryption.Validate(requestCookie, store.CookieOptions.CookieSecret, store.CookieOptions.CookieExpire) if !ok { - return fmt.Errorf("Cookie Signature not valid") + return fmt.Errorf("cookie signature not valid") } // We only return an error if we had an issue with redis diff --git a/providers/bitbucket.go b/providers/bitbucket.go index 7ddb280e..f67e48bc 100644 --- a/providers/bitbucket.go +++ b/providers/bitbucket.go @@ -116,7 +116,7 @@ func (p *BitbucketProvider) GetEmailAddress(s *sessions.SessionState) (string, e break } } - if found != true { + if !found { logger.Print("team membership test failed, access denied") return "", nil } @@ -147,7 +147,7 @@ func (p *BitbucketProvider) GetEmailAddress(s *sessions.SessionState) (string, e break } } - if found != true { + if !found { logger.Print("repository access test failed, access denied") return "", nil } diff --git a/providers/github.go b/providers/github.go index bacda1bb..cb522b13 100644 --- a/providers/github.go +++ b/providers/github.go @@ -122,7 +122,7 @@ func (p *GitHubProvider) hasOrg(accessToken string) (bool, error) { pn++ } - var presentOrgs []string + presentOrgs := make([]string, 0, len(orgs)) for _, org := range orgs { if p.Org == org.Login { logger.Printf("Found Github Organization: %q", org.Login) diff --git a/providers/gitlab.go b/providers/gitlab.go index 20421d65..6034115c 100644 --- a/providers/gitlab.go +++ b/providers/gitlab.go @@ -222,11 +222,7 @@ func (p *GitLabProvider) createSessionState(ctx context.Context, token *oauth2.T func (p *GitLabProvider) ValidateSessionState(s *sessions.SessionState) bool { ctx := context.Background() _, err := p.Verifier.Verify(ctx, s.IDToken) - if err != nil { - return false - } - - return true + return err == nil } // GetEmailAddress returns the Account email address diff --git a/providers/google.go b/providers/google.go index 804ea345..a93b8e08 100644 --- a/providers/google.go +++ b/providers/google.go @@ -2,6 +2,7 @@ package providers import ( "bytes" + "context" "encoding/base64" "encoding/json" "errors" @@ -15,10 +16,10 @@ import ( "github.com/oauth2-proxy/oauth2-proxy/pkg/apis/sessions" "github.com/oauth2-proxy/oauth2-proxy/pkg/logger" - "golang.org/x/oauth2" "golang.org/x/oauth2/google" admin "google.golang.org/api/admin/directory/v1" "google.golang.org/api/googleapi" + "google.golang.org/api/option" ) // GoogleProvider represents an Google based Identity Provider @@ -184,8 +185,9 @@ func getAdminService(adminEmail string, credentialsReader io.Reader) *admin.Serv } conf.Subject = adminEmail - client := conf.Client(oauth2.NoContext) - adminService, err := admin.New(client) + ctx := context.Background() + client := conf.Client(ctx) + adminService, err := admin.NewService(ctx, option.WithHTTPClient(client)) if err != nil { logger.Fatal(err) } diff --git a/providers/keycloak.go b/providers/keycloak.go index 9475957d..d8d41801 100644 --- a/providers/keycloak.go +++ b/providers/keycloak.go @@ -76,7 +76,7 @@ func (p *KeycloakProvider) GetEmailAddress(s *sessions.SessionState) (string, er } } - if found != true { + if !found { logger.Printf("group not found, access denied") return "", nil } diff --git a/providers/logingov.go b/providers/logingov.go index db112aa5..d5a34a41 100644 --- a/providers/logingov.go +++ b/providers/logingov.go @@ -183,7 +183,7 @@ func (p *LoginGovProvider) Redeem(redirectURL, code string) (s *sessions.Session Issuer: p.ClientID, Subject: p.ClientID, Audience: p.RedeemURL.String(), - ExpiresAt: int64(time.Now().Add(time.Duration(5 * time.Minute)).Unix()), + ExpiresAt: time.Now().Add(5 * time.Minute).Unix(), Id: randSeq(32), } token := jwt.NewWithClaims(jwt.GetSigningMethod("RS256"), claims) @@ -260,8 +260,7 @@ func (p *LoginGovProvider) Redeem(redirectURL, code string) (s *sessions.Session // GetLoginURL overrides GetLoginURL to add login.gov parameters func (p *LoginGovProvider) GetLoginURL(redirectURI, state string) string { - var a url.URL - a = *p.LoginURL + a := *p.LoginURL params, _ := url.ParseQuery(a.RawQuery) params.Set("redirect_uri", redirectURI) params.Set("approval_prompt", p.ApprovalPrompt) diff --git a/providers/oidc.go b/providers/oidc.go index e831ccba..ed982e1a 100644 --- a/providers/oidc.go +++ b/providers/oidc.go @@ -180,11 +180,7 @@ func (p *OIDCProvider) createSessionState(token *oauth2.Token, idToken *oidc.IDT func (p *OIDCProvider) ValidateSessionState(s *sessions.SessionState) bool { ctx := context.Background() _, err := p.Verifier.Verify(ctx, s.IDToken) - if err != nil { - return false - } - - return true + return err == nil } func getOIDCHeader(accessToken string) http.Header { diff --git a/providers/oidc_test.go b/providers/oidc_test.go index 7a0581a9..389568bf 100644 --- a/providers/oidc_test.go +++ b/providers/oidc_test.go @@ -62,6 +62,9 @@ type fakeKeySetStub struct{} func (fakeKeySetStub) VerifySignature(_ context.Context, jwt string) (payload []byte, err error) { decodeString, err := base64.RawURLEncoding.DecodeString(strings.Split(jwt, ".")[1]) + if err != nil { + return nil, err + } tokenClaims := &idTokenClaims{} err = json.Unmarshal(decodeString, tokenClaims) @@ -242,7 +245,9 @@ func TestOIDCProvider_findVerifiedIdToken(t *testing.T) { verifiedIDToken, err := provider.findVerifiedIDToken(context.Background(), tokenWithIDToken) assert.Equal(t, true, err == nil) - assert.Equal(t, true, verifiedIDToken != nil) + if verifiedIDToken == nil { + t.Fatal("verifiedIDToken is nil") + } assert.Equal(t, defaultIDToken.Issuer, verifiedIDToken.Issuer) assert.Equal(t, defaultIDToken.Subject, verifiedIDToken.Subject) diff --git a/providers/provider_data.go b/providers/provider_data.go index ce80c8b7..de5bc0d3 100644 --- a/providers/provider_data.go +++ b/providers/provider_data.go @@ -31,7 +31,7 @@ type ProviderData struct { // Data returns the ProviderData func (p *ProviderData) Data() *ProviderData { return p } -func (p *ProviderData) GetClientSecret() (ClientSecret string, err error) { +func (p *ProviderData) GetClientSecret() (clientSecret string, err error) { if p.ClientSecret != "" || p.ClientSecretFile == "" { return p.ClientSecret, nil } diff --git a/providers/provider_default.go b/providers/provider_default.go index 707e3a6e..1daf6d53 100644 --- a/providers/provider_default.go +++ b/providers/provider_default.go @@ -86,8 +86,7 @@ func (p *ProviderData) Redeem(redirectURL, code string) (s *sessions.SessionStat // GetLoginURL with typical oauth parameters func (p *ProviderData) GetLoginURL(redirectURI, state string) string { - var a url.URL - a = *p.LoginURL + a := *p.LoginURL params, _ := url.ParseQuery(a.RawQuery) params.Set("redirect_uri", redirectURI) params.Add("acr_values", p.AcrValues) diff --git a/watcher.go b/watcher.go index c1ca8b3d..7c916235 100644 --- a/watcher.go +++ b/watcher.go @@ -44,7 +44,7 @@ func WatchForUpdates(filename string, done <-chan bool, action func()) { defer watcher.Close() for { select { - case _ = <-done: + case <-done: logger.Printf("Shutting down watcher for: %s", filename) return case event := <-watcher.Events: