From 8c3dc0ca66e86d0af52bc0f056aaf0ddb3fe8c2a Mon Sep 17 00:00:00 2001 From: Mark Sagi-Kazar Date: Wed, 18 Dec 2019 15:46:49 +0100 Subject: [PATCH 1/8] Remove unused code (fixed: unused, structcheck, deadcode linters) --- .golangci.yml | 3 --- connector/google/google.go | 1 - server/templates.go | 13 ------------- 3 files changed, 17 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 824484d6..45cfc35f 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -16,10 +16,7 @@ linters: - wsl # TODO: fix me - - unused - - structcheck - stylecheck - - deadcode - misspell - unparam - goimports diff --git a/connector/google/google.go b/connector/google/google.go index 84e5580d..7561ca4d 100644 --- a/connector/google/google.go +++ b/connector/google/google.go @@ -105,7 +105,6 @@ type googleConnector struct { redirectURI string oauth2Config *oauth2.Config verifier *oidc.IDTokenVerifier - ctx context.Context cancel context.CancelFunc logger log.Logger hostedDomains []string diff --git a/server/templates.go b/server/templates.go index 88aeace0..13dda8f1 100644 --- a/server/templates.go +++ b/server/templates.go @@ -47,19 +47,6 @@ type webConfig struct { extra map[string]string } -func join(base, path string) string { - b := strings.HasSuffix(base, "/") - p := strings.HasPrefix(path, "/") - switch { - case b && p: - return base + path[1:] - case b || p: - return base + path - default: - return base + "/" + path - } -} - func dirExists(dir string) error { stat, err := os.Stat(dir) if err != nil { From 142c96c210a8a5b856b91c79b148ff8e59eaf448 Mon Sep 17 00:00:00 2001 From: Mark Sagi-Kazar Date: Wed, 18 Dec 2019 15:50:36 +0100 Subject: [PATCH 2/8] Fix stylecheck --- .golangci.yml | 1 - cmd/example-app/main.go | 4 +-- connector/saml/saml.go | 8 ++--- storage/kubernetes/client.go | 58 ++++++++++++++++++------------------ 4 files changed, 35 insertions(+), 36 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 45cfc35f..5bfe3e99 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -16,7 +16,6 @@ linters: - wsl # TODO: fix me - - stylecheck - misspell - unparam - goimports diff --git a/cmd/example-app/main.go b/cmd/example-app/main.go index a21b2e86..c16ddfdf 100644 --- a/cmd/example-app/main.go +++ b/cmd/example-app/main.go @@ -143,7 +143,7 @@ func cmd() *cobra.Command { ctx := oidc.ClientContext(context.Background(), a.client) provider, err := oidc.NewProvider(ctx, issuerURL) if err != nil { - return fmt.Errorf("Failed to query provider %q: %v", issuerURL, err) + return fmt.Errorf("failed to query provider %q: %v", issuerURL, err) } var s struct { @@ -153,7 +153,7 @@ func cmd() *cobra.Command { ScopesSupported []string `json:"scopes_supported"` } if err := provider.Claims(&s); err != nil { - return fmt.Errorf("Failed to parse provider scopes_supported: %v", err) + return fmt.Errorf("failed to parse provider scopes_supported: %v", err) } if len(s.ScopesSupported) == 0 { diff --git a/connector/saml/saml.go b/connector/saml/saml.go index 09414ea3..f4e52a69 100644 --- a/connector/saml/saml.go +++ b/connector/saml/saml.go @@ -325,7 +325,7 @@ func (p *provider) HandlePOST(s connector.Scopes, samlResponse, inResponseTo str // Status is a required element. if resp.Status == nil { - return ident, fmt.Errorf("Response did not contain a Status element") + return ident, fmt.Errorf("response did not contain a Status element") } if err = p.validateStatus(resp.Status); err != nil { @@ -398,7 +398,7 @@ func (p *provider) HandlePOST(s connector.Scopes, samlResponse, inResponseTo str if len(p.allowedGroups) > 0 && (!s.Groups || p.groupsAttr == "") { // allowedGroups set but no groups or groupsAttr. Disallowing. - return ident, fmt.Errorf("User not a member of allowed groups") + return ident, fmt.Errorf("user not a member of allowed groups") } // Grab the groups. @@ -427,7 +427,7 @@ func (p *provider) HandlePOST(s connector.Scopes, samlResponse, inResponseTo str if len(groupMatches) == 0 { // No group membership matches found, disallowing - return ident, fmt.Errorf("User not a member of allowed groups") + return ident, fmt.Errorf("user not a member of allowed groups") } // Otherwise, we're good @@ -468,7 +468,7 @@ func (p *provider) validateStatus(status *status) error { func (p *provider) validateSubject(subject *subject, inResponseTo string) error { // Optional according to the spec, but again, we're going to be strict here. if len(subject.SubjectConfirmations) == 0 { - return fmt.Errorf("Subject contained no SubjectConfirmations") + return fmt.Errorf("subject contained no SubjectConfirmations") } var errs []error diff --git a/storage/kubernetes/client.go b/storage/kubernetes/client.go index f2af0e57..795dcc22 100644 --- a/storage/kubernetes/client.go +++ b/storage/kubernetes/client.go @@ -55,14 +55,14 @@ type client struct { } // idToName maps an arbitrary ID, such as an email or client ID to a Kubernetes object name. -func (c *client) idToName(s string) string { - return idToName(s, c.hash) +func (cli *client) idToName(s string) string { + return idToName(s, cli.hash) } // offlineTokenName maps two arbitrary IDs, to a single Kubernetes object name. // This is used when more than one field is used to uniquely identify the object. -func (c *client) offlineTokenName(userID string, connID string) string { - return offlineTokenName(userID, connID, c.hash) +func (cli *client) offlineTokenName(userID string, connID string) string { + return offlineTokenName(userID, connID, cli.hash) } // Kubernetes names must match the regexp '[a-z0-9]([-a-z0-9]*[a-z0-9])?'. @@ -79,7 +79,7 @@ func offlineTokenName(userID string, connID string, h func() hash.Hash) string { return strings.TrimRight(encoding.EncodeToString(hash.Sum(nil)), "=") } -func (c *client) urlFor(apiVersion, namespace, resource, name string) string { +func (cli *client) urlFor(apiVersion, namespace, resource, name string) string { basePath := "apis/" if apiVersion == "v1" { basePath = "api/" @@ -91,10 +91,10 @@ func (c *client) urlFor(apiVersion, namespace, resource, name string) string { } else { p = path.Join(basePath, apiVersion, resource, name) } - if strings.HasSuffix(c.baseURL, "/") { - return c.baseURL + p + if strings.HasSuffix(cli.baseURL, "/") { + return cli.baseURL + p } - return c.baseURL + "/" + p + return cli.baseURL + "/" + p } // Define an error interface so we can get at the underlying status code if it's @@ -156,13 +156,13 @@ func closeResp(r *http.Response) { r.Body.Close() } -func (c *client) get(resource, name string, v interface{}) error { - return c.getResource(c.apiVersion, c.namespace, resource, name, v) +func (cli *client) get(resource, name string, v interface{}) error { + return cli.getResource(cli.apiVersion, cli.namespace, resource, name, v) } -func (c *client) getResource(apiVersion, namespace, resource, name string, v interface{}) error { - url := c.urlFor(apiVersion, namespace, resource, name) - resp, err := c.client.Get(url) +func (cli *client) getResource(apiVersion, namespace, resource, name string, v interface{}) error { + url := cli.urlFor(apiVersion, namespace, resource, name) + resp, err := cli.client.Get(url) if err != nil { return err } @@ -173,22 +173,22 @@ func (c *client) getResource(apiVersion, namespace, resource, name string, v int return json.NewDecoder(resp.Body).Decode(v) } -func (c *client) list(resource string, v interface{}) error { - return c.get(resource, "", v) +func (cli *client) list(resource string, v interface{}) error { + return cli.get(resource, "", v) } -func (c *client) post(resource string, v interface{}) error { - return c.postResource(c.apiVersion, c.namespace, resource, v) +func (cli *client) post(resource string, v interface{}) error { + return cli.postResource(cli.apiVersion, cli.namespace, resource, v) } -func (c *client) postResource(apiVersion, namespace, resource string, v interface{}) error { +func (cli *client) postResource(apiVersion, namespace, resource string, v interface{}) error { body, err := json.Marshal(v) if err != nil { return fmt.Errorf("marshal object: %v", err) } - url := c.urlFor(apiVersion, namespace, resource, "") - resp, err := c.client.Post(url, "application/json", bytes.NewReader(body)) + url := cli.urlFor(apiVersion, namespace, resource, "") + resp, err := cli.client.Post(url, "application/json", bytes.NewReader(body)) if err != nil { return err } @@ -196,13 +196,13 @@ func (c *client) postResource(apiVersion, namespace, resource string, v interfac return checkHTTPErr(resp, http.StatusCreated) } -func (c *client) delete(resource, name string) error { - url := c.urlFor(c.apiVersion, c.namespace, resource, name) +func (cli *client) delete(resource, name string) error { + url := cli.urlFor(cli.apiVersion, cli.namespace, resource, name) req, err := http.NewRequest("DELETE", url, nil) if err != nil { return fmt.Errorf("create delete request: %v", err) } - resp, err := c.client.Do(req) + resp, err := cli.client.Do(req) if err != nil { return fmt.Errorf("delete request: %v", err) } @@ -210,7 +210,7 @@ func (c *client) delete(resource, name string) error { return checkHTTPErr(resp, http.StatusOK) } -func (c *client) deleteAll(resource string) error { +func (cli *client) deleteAll(resource string) error { var list struct { k8sapi.TypeMeta `json:",inline"` k8sapi.ListMeta `json:"metadata,omitempty"` @@ -219,24 +219,24 @@ func (c *client) deleteAll(resource string) error { k8sapi.ObjectMeta `json:"metadata,omitempty"` } `json:"items"` } - if err := c.list(resource, &list); err != nil { + if err := cli.list(resource, &list); err != nil { return err } for _, item := range list.Items { - if err := c.delete(resource, item.Name); err != nil { + if err := cli.delete(resource, item.Name); err != nil { return err } } return nil } -func (c *client) put(resource, name string, v interface{}) error { +func (cli *client) put(resource, name string, v interface{}) error { body, err := json.Marshal(v) if err != nil { return fmt.Errorf("marshal object: %v", err) } - url := c.urlFor(c.apiVersion, c.namespace, resource, name) + url := cli.urlFor(cli.apiVersion, cli.namespace, resource, name) req, err := http.NewRequest("PUT", url, bytes.NewReader(body)) if err != nil { return fmt.Errorf("create patch request: %v", err) @@ -244,7 +244,7 @@ func (c *client) put(resource, name string, v interface{}) error { req.Header.Set("Content-Length", strconv.Itoa(len(body))) - resp, err := c.client.Do(req) + resp, err := cli.client.Do(req) if err != nil { return fmt.Errorf("patch request: %v", err) } From 367b187cf4d529ef57f457c99ae7ad53700e9c19 Mon Sep 17 00:00:00 2001 From: Mark Sagi-Kazar Date: Wed, 18 Dec 2019 15:51:44 +0100 Subject: [PATCH 3/8] Fix missspell --- .golangci.yml | 1 - connector/github/github.go | 4 ++-- server/handlers.go | 4 ++-- storage/kubernetes/k8sapi/crd_extensions.go | 2 +- storage/sql/config.go | 2 +- 5 files changed, 6 insertions(+), 7 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 5bfe3e99..7ca89f0c 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -16,7 +16,6 @@ linters: - wsl # TODO: fix me - - misspell - unparam - goimports - golint diff --git a/connector/github/github.go b/connector/github/github.go index 6d915edc..d9356e73 100644 --- a/connector/github/github.go +++ b/connector/github/github.go @@ -144,7 +144,7 @@ type githubConnector struct { hostName string // Used to support untrusted/self-signed CA certs. rootCA string - // HTTP Client that trusts the custom delcared rootCA cert. + // HTTP Client that trusts the custom declared rootCA cert. httpClient *http.Client // optional choice between 'name' (default) or 'slug' teamNameField string @@ -206,7 +206,7 @@ func (e *oauth2Error) Error() string { return e.error + ": " + e.errorDescription } -// newHTTPClient returns a new HTTP client that trusts the custom delcared rootCA cert. +// newHTTPClient returns a new HTTP client that trusts the custom declared rootCA cert. func newHTTPClient(rootCA string) (*http.Client, error) { tlsConfig := tls.Config{RootCAs: x509.NewCertPool()} rootCABytes, err := ioutil.ReadFile(rootCA) diff --git a/server/handlers.go b/server/handlers.go index 0f5b0d23..ede474a4 100644 --- a/server/handlers.go +++ b/server/handlers.go @@ -45,8 +45,8 @@ func (s *Server) newHealthChecker(ctx context.Context) http.Handler { return h } -// healthChecker periodically performs health checks on server dependenices. -// Currently, it only checks that the storage layer is avialable. +// healthChecker periodically performs health checks on server dependencies. +// Currently, it only checks that the storage layer is available. type healthChecker struct { s *Server diff --git a/storage/kubernetes/k8sapi/crd_extensions.go b/storage/kubernetes/k8sapi/crd_extensions.go index 4b55e393..7a65410b 100644 --- a/storage/kubernetes/k8sapi/crd_extensions.go +++ b/storage/kubernetes/k8sapi/crd_extensions.go @@ -43,7 +43,7 @@ type CustomResourceDefinitionNames struct { ListKind string `json:"listKind,omitempty" protobuf:"bytes,5,opt,name=listKind"` } -// ResourceScope is an enum defining the different scopes availabe to a custom resource +// ResourceScope is an enum defining the different scopes available to a custom resource type ResourceScope string const ( diff --git a/storage/sql/config.go b/storage/sql/config.go index 24467169..827b0df1 100644 --- a/storage/sql/config.go +++ b/storage/sql/config.go @@ -311,7 +311,7 @@ func (s *MySQL) open(logger log.Logger) (*conn, error) { err = db.Ping() if err != nil { if mysqlErr, ok := err.(*mysql.MySQLError); ok && mysqlErr.Number == mysqlErrUnknownSysVar { - logger.Info("reconnecting with MySQL pre-5.7.20 compatibilty mode") + logger.Info("reconnecting with MySQL pre-5.7.20 compatibility mode") // MySQL 5.7.20 introduced transaction_isolation and deprecated tx_isolation. // MySQL 8.0 doesn't have tx_isolation at all. From 9bd5ae519739254ee75baa5f8507b6a18b39bfdf Mon Sep 17 00:00:00 2001 From: Mark Sagi-Kazar Date: Wed, 18 Dec 2019 15:53:34 +0100 Subject: [PATCH 4/8] Fix goimports --- .golangci.yml | 1 - cmd/dex/config_test.go | 2 +- connector/google/google.go | 4 ++-- connector/oidc/oidc_test.go | 3 ++- connector/saml/saml.go | 5 +++-- server/server.go | 8 ++++---- 6 files changed, 12 insertions(+), 11 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 7ca89f0c..c658d914 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -17,7 +17,6 @@ linters: # TODO: fix me - unparam - - goimports - golint - whitespace - goconst diff --git a/cmd/dex/config_test.go b/cmd/dex/config_test.go index d7875ad1..87f1a578 100644 --- a/cmd/dex/config_test.go +++ b/cmd/dex/config_test.go @@ -1,7 +1,6 @@ package main import ( - "github.com/dexidp/dex/server" "testing" "github.com/ghodss/yaml" @@ -9,6 +8,7 @@ import ( "github.com/dexidp/dex/connector/mock" "github.com/dexidp/dex/connector/oidc" + "github.com/dexidp/dex/server" "github.com/dexidp/dex/storage" "github.com/dexidp/dex/storage/sql" ) diff --git a/connector/google/google.go b/connector/google/google.go index 7561ca4d..37b6dc15 100644 --- a/connector/google/google.go +++ b/connector/google/google.go @@ -11,12 +11,12 @@ import ( "github.com/coreos/go-oidc" "golang.org/x/oauth2" + "golang.org/x/oauth2/google" + admin "google.golang.org/api/admin/directory/v1" "github.com/dexidp/dex/connector" pkg_groups "github.com/dexidp/dex/pkg/groups" "github.com/dexidp/dex/pkg/log" - "golang.org/x/oauth2/google" - admin "google.golang.org/api/admin/directory/v1" ) const ( diff --git a/connector/oidc/oidc_test.go b/connector/oidc/oidc_test.go index f766a875..22b035c1 100644 --- a/connector/oidc/oidc_test.go +++ b/connector/oidc/oidc_test.go @@ -16,9 +16,10 @@ import ( "testing" "time" - "github.com/dexidp/dex/connector" "github.com/sirupsen/logrus" "gopkg.in/square/go-jose.v2" + + "github.com/dexidp/dex/connector" ) func TestKnownBrokenAuthHeaderProvider(t *testing.T) { diff --git a/connector/saml/saml.go b/connector/saml/saml.go index f4e52a69..70ab413c 100644 --- a/connector/saml/saml.go +++ b/connector/saml/saml.go @@ -14,11 +14,12 @@ import ( "time" "github.com/beevik/etree" + dsig "github.com/russellhaering/goxmldsig" + "github.com/russellhaering/goxmldsig/etreeutils" + "github.com/dexidp/dex/connector" "github.com/dexidp/dex/pkg/groups" "github.com/dexidp/dex/pkg/log" - dsig "github.com/russellhaering/goxmldsig" - "github.com/russellhaering/goxmldsig/etreeutils" ) // nolint diff --git a/server/server.go b/server/server.go index fa860330..4dc7337d 100644 --- a/server/server.go +++ b/server/server.go @@ -14,6 +14,10 @@ import ( "sync/atomic" "time" + "github.com/felixge/httpsnoop" + "github.com/gorilla/handlers" + "github.com/gorilla/mux" + "github.com/prometheus/client_golang/prometheus" "golang.org/x/crypto/bcrypt" "github.com/dexidp/dex/connector" @@ -31,10 +35,6 @@ import ( "github.com/dexidp/dex/connector/saml" "github.com/dexidp/dex/pkg/log" "github.com/dexidp/dex/storage" - "github.com/felixge/httpsnoop" - "github.com/gorilla/handlers" - "github.com/gorilla/mux" - "github.com/prometheus/client_golang/prometheus" ) // LocalConnector is the local passwordDB connector which is an internal From f141f2133b0e9e2ab602d3e41cddf57c0d659b54 Mon Sep 17 00:00:00 2001 From: Mark Sagi-Kazar Date: Wed, 18 Dec 2019 15:56:12 +0100 Subject: [PATCH 5/8] Fix whitespace --- .golangci.yml | 1 - cmd/dex/config_test.go | 1 - cmd/dex/serve.go | 1 - connector/bitbucketcloud/bitbucketcloud.go | 2 -- connector/bitbucketcloud/bitbucketcloud_test.go | 3 --- connector/github/github.go | 2 -- connector/github/github_test.go | 2 -- connector/gitlab/gitlab_test.go | 4 ---- connector/keystone/keystone.go | 1 - connector/ldap/ldap.go | 2 -- connector/saml/saml.go | 1 - server/api.go | 2 -- server/api_test.go | 2 -- server/handlers.go | 1 - server/handlers_test.go | 1 - server/server_test.go | 3 --- server/templates.go | 1 - storage/conformance/conformance.go | 2 -- storage/sql/crud.go | 1 - 19 files changed, 33 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index c658d914..d0dcfd9b 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -18,7 +18,6 @@ linters: # TODO: fix me - unparam - golint - - whitespace - goconst - unconvert - bodyclose diff --git a/cmd/dex/config_test.go b/cmd/dex/config_test.go index 87f1a578..1df1f809 100644 --- a/cmd/dex/config_test.go +++ b/cmd/dex/config_test.go @@ -211,5 +211,4 @@ logger: if diff := pretty.Compare(c, want); diff != "" { t.Errorf("got!=want: %s", diff) } - } diff --git a/cmd/dex/serve.go b/cmd/dex/serve.go index 7d0e0deb..293d3e66 100644 --- a/cmd/dex/serve.go +++ b/cmd/dex/serve.go @@ -182,7 +182,6 @@ func serve(cmd *cobra.Command, args []string) error { return fmt.Errorf("failed to initialize storage connectors: %v", err) } storageConnectors[i] = conn - } if c.EnablePasswordDB { diff --git a/connector/bitbucketcloud/bitbucketcloud.go b/connector/bitbucketcloud/bitbucketcloud.go index 7d58221a..d8ccf2e5 100644 --- a/connector/bitbucketcloud/bitbucketcloud.go +++ b/connector/bitbucketcloud/bitbucketcloud.go @@ -41,7 +41,6 @@ type Config struct { // Open returns a strategy for logging in through Bitbucket. func (c *Config) Open(id string, logger log.Logger) (connector.Connector, error) { - b := bitbucketConnector{ redirectURI: c.RedirectURI, teams: c.Teams, @@ -373,7 +372,6 @@ type userTeamsResponse struct { } func (b *bitbucketConnector) userTeams(ctx context.Context, client *http.Client) ([]string, error) { - var teams []string apiURL := b.apiURL + "/teams?role=member" diff --git a/connector/bitbucketcloud/bitbucketcloud_test.go b/connector/bitbucketcloud/bitbucketcloud_test.go index b9f4ba08..488c7886 100644 --- a/connector/bitbucketcloud/bitbucketcloud_test.go +++ b/connector/bitbucketcloud/bitbucketcloud_test.go @@ -14,7 +14,6 @@ import ( ) func TestUserGroups(t *testing.T) { - teamsResponse := userTeamsResponse{ pagedResponse: pagedResponse{ Size: 3, @@ -46,7 +45,6 @@ func TestUserGroups(t *testing.T) { } func TestUserWithoutTeams(t *testing.T) { - s := newTestServer(map[string]interface{}{ "/teams?role=member": userTeamsResponse{}, }) @@ -61,7 +59,6 @@ func TestUserWithoutTeams(t *testing.T) { } func TestUsernameIncludedInFederatedIdentity(t *testing.T) { - s := newTestServer(map[string]interface{}{ "/user": user{Username: "some-login"}, "/user/emails": userEmailResponse{ diff --git a/connector/github/github.go b/connector/github/github.go index d9356e73..41aaf589 100644 --- a/connector/github/github.go +++ b/connector/github/github.go @@ -67,7 +67,6 @@ type Org struct { // Open returns a strategy for logging in through GitHub. func (c *Config) Open(id string, logger log.Logger) (connector.Connector, error) { - if c.Org != "" { // Return error if both 'org' and 'orgs' fields are used. if len(c.Orgs) > 0 { @@ -107,7 +106,6 @@ func (c *Config) Open(id string, logger log.Logger) (connector.Connector, error) if g.httpClient, err = newHTTPClient(g.rootCA); err != nil { return nil, fmt.Errorf("failed to create HTTP client: %v", err) } - } g.loadAllGroups = c.LoadAllGroups diff --git a/connector/github/github_test.go b/connector/github/github_test.go index 539a2e69..76d7463c 100644 --- a/connector/github/github_test.go +++ b/connector/github/github_test.go @@ -126,7 +126,6 @@ func TestUserGroupsWithTeamNameAndSlugFieldConfig(t *testing.T) { // tests that the users login is used as their username when they have no username set func TestUsernameIncludedInFederatedIdentity(t *testing.T) { - s := newTestServer(map[string]testResponse{ "/user": {data: user{Login: "some-login", ID: 12345678}}, "/user/emails": {data: []userEmail{{ @@ -168,7 +167,6 @@ func TestUsernameIncludedInFederatedIdentity(t *testing.T) { } func TestLoginUsedAsIDWhenConfigured(t *testing.T) { - s := newTestServer(map[string]testResponse{ "/user": {data: user{Login: "some-login", ID: 12345678, Name: "Joe Bloggs"}}, "/user/emails": {data: []userEmail{{ diff --git a/connector/gitlab/gitlab_test.go b/connector/gitlab/gitlab_test.go index 331b486e..23cf9aac 100644 --- a/connector/gitlab/gitlab_test.go +++ b/connector/gitlab/gitlab_test.go @@ -65,7 +65,6 @@ func TestUserGroupsWithoutOrgs(t *testing.T) { // tests that the email is used as their username when they have no username set func TestUsernameIncludedInFederatedIdentity(t *testing.T) { - s := newTestServer(map[string]interface{}{ "/api/v4/user": gitlabUser{Email: "some@email.com", ID: 12345678}, "/oauth/token": map[string]interface{}{ @@ -102,7 +101,6 @@ func TestUsernameIncludedInFederatedIdentity(t *testing.T) { } func TestLoginUsedAsIDWhenConfigured(t *testing.T) { - s := newTestServer(map[string]interface{}{ "/api/v4/user": gitlabUser{Email: "some@email.com", ID: 12345678, Name: "Joe Bloggs", Username: "joebloggs"}, "/oauth/token": map[string]interface{}{ @@ -130,7 +128,6 @@ func TestLoginUsedAsIDWhenConfigured(t *testing.T) { } func TestLoginWithTeamWhitelisted(t *testing.T) { - s := newTestServer(map[string]interface{}{ "/api/v4/user": gitlabUser{Email: "some@email.com", ID: 12345678, Name: "Joe Bloggs"}, "/oauth/token": map[string]interface{}{ @@ -158,7 +155,6 @@ func TestLoginWithTeamWhitelisted(t *testing.T) { } func TestLoginWithTeamNonWhitelisted(t *testing.T) { - s := newTestServer(map[string]interface{}{ "/api/v4/user": gitlabUser{Email: "some@email.com", ID: 12345678, Name: "Joe Bloggs", Username: "joebloggs"}, "/oauth/token": map[string]interface{}{ diff --git a/connector/keystone/keystone.go b/connector/keystone/keystone.go index dc74a01f..71a59de8 100644 --- a/connector/keystone/keystone.go +++ b/connector/keystone/keystone.go @@ -150,7 +150,6 @@ func (p *conn) Prompt() string { return "username" } func (p *conn) Refresh( ctx context.Context, scopes connector.Scopes, identity connector.Identity) (connector.Identity, error) { - token, err := p.getAdminToken(ctx) if err != nil { return identity, fmt.Errorf("keystone: failed to obtain admin token: %v", err) diff --git a/connector/ldap/ldap.go b/connector/ldap/ldap.go index aed73194..cac3539f 100644 --- a/connector/ldap/ldap.go +++ b/connector/ldap/ldap.go @@ -189,7 +189,6 @@ func (c *Config) OpenConnector(logger log.Logger) (interface { } func (c *Config) openConnector(logger log.Logger) (*ldapConnector, error) { - requiredFields := []struct { name string val string @@ -365,7 +364,6 @@ func (c *ldapConnector) identityFromEntry(user ldap.Entry) (ident connector.Iden } func (c *ldapConnector) userEntry(conn *ldap.Conn, username string) (user ldap.Entry, found bool, err error) { - filter := fmt.Sprintf("(%s=%s)", c.UserSearch.Username, ldap.EscapeFilter(username)) if c.UserSearch.Filter != "" { filter = fmt.Sprintf("(&%s%s)", c.UserSearch.Filter, filter) diff --git a/connector/saml/saml.go b/connector/saml/saml.go index 70ab413c..1c1cb460 100644 --- a/connector/saml/saml.go +++ b/connector/saml/saml.go @@ -249,7 +249,6 @@ type provider struct { } func (p *provider) POSTData(s connector.Scopes, id string) (action, value string, err error) { - r := &authnRequest{ ProtocolBinding: bindingPOST, ID: id, diff --git a/server/api.go b/server/api.go index 9feb0e9a..55784b90 100644 --- a/server/api.go +++ b/server/api.go @@ -218,7 +218,6 @@ func (d dexAPI) DeletePassword(ctx context.Context, req *api.DeletePasswordReq) return nil, fmt.Errorf("delete password: %v", err) } return &api.DeletePasswordResp{}, nil - } func (d dexAPI) GetVersion(ctx context.Context, req *api.VersionReq) (*api.VersionResp, error) { @@ -248,7 +247,6 @@ func (d dexAPI) ListPasswords(ctx context.Context, req *api.ListPasswordReq) (*a return &api.ListPasswordResp{ Passwords: passwords, }, nil - } func (d dexAPI) VerifyPassword(ctx context.Context, req *api.VerifyPasswordReq) (*api.VerifyPasswordResp, error) { diff --git a/server/api_test.go b/server/api_test.go index 80a22486..511d5d39 100644 --- a/server/api_test.go +++ b/server/api_test.go @@ -167,7 +167,6 @@ func TestPassword(t *testing.T) { if _, err := client.DeletePassword(ctx, &deleteReq); err != nil { t.Fatalf("Unable to delete password: %v", err) } - } // Ensures checkCost returns expected values @@ -495,7 +494,6 @@ func TestUpdateClient(t *testing.T) { if tc.cleanup != nil { tc.cleanup(t, tc.req.Id) } - }) } } diff --git a/server/handlers.go b/server/handlers.go index ede474a4..97a46d57 100644 --- a/server/handlers.go +++ b/server/handlers.go @@ -922,7 +922,6 @@ func (s *Server) handleAuthCode(w http.ResponseWriter, r *http.Request, client s deleteToken = true return } - } } s.writeAccessToken(w, idToken, accessToken, refreshToken, expiry) diff --git a/server/handlers_test.go b/server/handlers_test.go index 395b7e72..b30076dd 100644 --- a/server/handlers_test.go +++ b/server/handlers_test.go @@ -24,7 +24,6 @@ func TestHandleHealth(t *testing.T) { if rr.Code != http.StatusOK { t.Errorf("expected 200 got %d", rr.Code) } - } type badStorage struct { diff --git a/server/server_test.go b/server/server_test.go index 6759f240..ef49ed6f 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -799,7 +799,6 @@ func TestCrossClientScopes(t *testing.T) { if !reflect.DeepEqual(idToken.Audience, expAudience) { t.Errorf("expected audience %q, got %q", expAudience, idToken.Audience) } - } if gotState := q.Get("state"); gotState != state { t.Errorf("state did not match, want=%q got=%q", state, gotState) @@ -921,7 +920,6 @@ func TestCrossClientScopesWithAzpInAudienceByDefault(t *testing.T) { if !reflect.DeepEqual(idToken.Audience, expAudience) { t.Errorf("expected audience %q, got %q", expAudience, idToken.Audience) } - } if gotState := q.Get("state"); gotState != state { t.Errorf("state did not match, want=%q got=%q", state, gotState) @@ -1058,7 +1056,6 @@ func TestPasswordDB(t *testing.T) { t.Errorf("%s: %s", tc.name, diff) } } - } func TestPasswordDBUsernamePrompt(t *testing.T) { diff --git a/server/templates.go b/server/templates.go index 13dda8f1..ac0957af 100644 --- a/server/templates.go +++ b/server/templates.go @@ -176,7 +176,6 @@ func loadTemplates(c webConfig, templatesDir string) (*templates, error) { //assetPath is static/main.css //relativeURL("/dex", "/dex/auth", "static/main.css") = "../static/main.css" func relativeURL(serverPath, reqPath, assetPath string) string { - splitPath := func(p string) []string { res := []string{} parts := strings.Split(path.Clean(p), "/") diff --git a/storage/conformance/conformance.go b/storage/conformance/conformance.go index 9832a7d8..92a48d6a 100644 --- a/storage/conformance/conformance.go +++ b/storage/conformance/conformance.go @@ -160,7 +160,6 @@ func testAuthRequestCRUD(t *testing.T, s storage.Storage) { if err := s.DeleteAuthRequest(a2.ID); err != nil { t.Fatalf("failed to delete auth request: %v", err) } - } func testAuthCodeCRUD(t *testing.T, s storage.Storage) { @@ -509,7 +508,6 @@ func testPasswordCRUD(t *testing.T, s storage.Storage) { _, err = s.GetPassword(password1.Email) mustBeErrNotFound(t, "password", err) - } func testOfflineSessionCRUD(t *testing.T, s storage.Storage) { diff --git a/storage/sql/crud.go b/storage/sql/crud.go index e96a7b12..e87dc56a 100644 --- a/storage/sql/crud.go +++ b/storage/sql/crud.go @@ -169,7 +169,6 @@ func (c *conn) UpdateAuthRequest(id string, updater func(a storage.AuthRequest) } return nil }) - } func (c *conn) GetAuthRequest(id string) (storage.AuthRequest, error) { From 2f8d1f8e42eebe6b4a4b421cc97053dcdb6b7b4c Mon Sep 17 00:00:00 2001 From: Mark Sagi-Kazar Date: Wed, 18 Dec 2019 15:56:46 +0100 Subject: [PATCH 6/8] Fix unconvert --- .golangci.yml | 1 - connector/keystone/keystone_test.go | 2 +- storage/etcd/etcd.go | 2 +- 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index d0dcfd9b..89e09982 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -19,7 +19,6 @@ linters: - unparam - golint - goconst - - unconvert - bodyclose - staticcheck - nakedret diff --git a/connector/keystone/keystone_test.go b/connector/keystone/keystone_test.go index d5d65ef1..d8bb0268 100644 --- a/connector/keystone/keystone_test.go +++ b/connector/keystone/keystone_test.go @@ -274,7 +274,7 @@ func TestUseRefreshToken(t *testing.T) { delete(t, token, groupID, groupsURL) expectEquals(t, 1, len(identityRefresh.Groups)) - expectEquals(t, testGroup, string(identityRefresh.Groups[0])) + expectEquals(t, testGroup, identityRefresh.Groups[0]) } func TestUseRefreshTokenUserDeleted(t *testing.T) { diff --git a/storage/etcd/etcd.go b/storage/etcd/etcd.go index d106f443..ee588129 100644 --- a/storage/etcd/etcd.go +++ b/storage/etcd/etcd.go @@ -156,7 +156,7 @@ func (c *conn) UpdateRefreshToken(id string, updater func(old storage.RefreshTok return c.txnUpdate(ctx, keyID(refreshTokenPrefix, id), func(currentValue []byte) ([]byte, error) { var current RefreshToken if len(currentValue) > 0 { - if err := json.Unmarshal([]byte(currentValue), ¤t); err != nil { + if err := json.Unmarshal(currentValue, ¤t); err != nil { return nil, err } } From 65c77e9db2e1e32fe4837970ea2da06758be42da Mon Sep 17 00:00:00 2001 From: Mark Sagi-Kazar Date: Wed, 18 Dec 2019 16:04:03 +0100 Subject: [PATCH 7/8] Fix bodyclose --- .golangci.yml | 1 - connector/keystone/keystone.go | 3 +++ connector/keystone/keystone_test.go | 15 +++++++++++++-- server/server_test.go | 16 ++++++++++++++-- 4 files changed, 30 insertions(+), 5 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 89e09982..1ff5dd64 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -19,7 +19,6 @@ linters: - unparam - golint - goconst - - bodyclose - staticcheck - nakedret - ineffassign diff --git a/connector/keystone/keystone.go b/connector/keystone/keystone.go index 71a59de8..6ce22d1a 100644 --- a/connector/keystone/keystone.go +++ b/connector/keystone/keystone.go @@ -209,6 +209,8 @@ func (p *conn) getAdminToken(ctx context.Context) (string, error) { if err != nil { return "", err } + defer resp.Body.Close() + token := resp.Header.Get("X-Subject-Token") return token, nil } @@ -228,6 +230,7 @@ func (p *conn) checkIfUserExists(ctx context.Context, userID string, token strin if err != nil { return false, err } + defer resp.Body.Close() if resp.StatusCode == 200 { return true, nil diff --git a/connector/keystone/keystone_test.go b/connector/keystone/keystone_test.go index d8bb0268..784cdc4f 100644 --- a/connector/keystone/keystone_test.go +++ b/connector/keystone/keystone_test.go @@ -154,7 +154,12 @@ func delete(t *testing.T, token, id, uri string) { t.Fatalf("error: %v", err) } req.Header.Set("X-Auth-Token", token) - client.Do(req) + + resp, err := client.Do(req) + if err != nil { + t.Fatalf("error: %v", err) + } + defer resp.Body.Close() } func createGroup(t *testing.T, token, description, name string) string { @@ -208,7 +213,13 @@ func addUserToGroup(t *testing.T, token, groupID, userID string) error { return err } req.Header.Set("X-Auth-Token", token) - client.Do(req) + + resp, err := client.Do(req) + if err != nil { + t.Fatalf("error: %v", err) + } + defer resp.Body.Close() + return nil } diff --git a/server/server_test.go b/server/server_test.go index ef49ed6f..8fe84c9a 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -590,6 +590,8 @@ func TestOAuth2CodeFlow(t *testing.T) { if err != nil { t.Fatalf("get failed: %v", err) } + defer resp.Body.Close() + if reqDump, err = httputil.DumpRequest(resp.Request, false); err != nil { t.Fatal(err) } @@ -726,6 +728,8 @@ func TestOAuth2ImplicitFlow(t *testing.T) { if err != nil { t.Fatalf("get failed: %v", err) } + defer resp.Body.Close() + if reqDump, err = httputil.DumpRequest(resp.Request, false); err != nil { t.Fatal(err) } @@ -847,6 +851,8 @@ func TestCrossClientScopes(t *testing.T) { if err != nil { t.Fatalf("get failed: %v", err) } + defer resp.Body.Close() + if reqDump, err = httputil.DumpRequest(resp.Request, false); err != nil { t.Fatal(err) } @@ -967,6 +973,8 @@ func TestCrossClientScopesWithAzpInAudienceByDefault(t *testing.T) { if err != nil { t.Fatalf("get failed: %v", err) } + defer resp.Body.Close() + if reqDump, err = httputil.DumpRequest(resp.Request, false); err != nil { t.Fatal(err) } @@ -1222,9 +1230,11 @@ func TestRefreshTokenFlow(t *testing.T) { RedirectURL: redirectURL, } - if _, err = http.Get(oauth2Client.server.URL + "/login"); err != nil { + resp, err := http.Get(oauth2Client.server.URL + "/login") + if err != nil { t.Fatalf("get failed: %v", err) } + defer resp.Body.Close() tok := &oauth2.Token{ RefreshToken: oauth2Client.token.RefreshToken, @@ -1232,9 +1242,11 @@ func TestRefreshTokenFlow(t *testing.T) { } // Login in again to receive a new token. - if _, err = http.Get(oauth2Client.server.URL + "/login"); err != nil { + resp, err = http.Get(oauth2Client.server.URL + "/login") + if err != nil { t.Fatalf("get failed: %v", err) } + defer resp.Body.Close() // try to refresh expired token with old refresh token. if _, err := oauth2Client.config.TokenSource(ctx, tok).Token(); err == nil { From 050d5af937626f5e1403aabff60ff07941dbc8a3 Mon Sep 17 00:00:00 2001 From: Mark Sagi-Kazar Date: Wed, 18 Dec 2019 16:07:06 +0100 Subject: [PATCH 8/8] Fix ineffassign --- .golangci.yml | 1 - server/templates.go | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/.golangci.yml b/.golangci.yml index 1ff5dd64..16d82a5e 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -21,7 +21,6 @@ linters: - goconst - staticcheck - nakedret - - ineffassign - errcheck - gosec - gochecknoinits diff --git a/server/templates.go b/server/templates.go index ac0957af..b9beab33 100644 --- a/server/templates.go +++ b/server/templates.go @@ -206,6 +206,7 @@ func relativeURL(serverPath, reqPath, assetPath string) string { server, req, asset := splitPath(serverPath), splitPath(reqPath), splitPath(assetPath) // Remove common prefix of request path with server path + // nolint: ineffassign server, req = stripCommonParts(server, req) // Remove common prefix of request path with asset path