From ed7b71a1906dd1fc013b5db17de144af96db6666 Mon Sep 17 00:00:00 2001 From: Mark Sagi-Kazar Date: Tue, 3 Nov 2020 20:37:38 +0100 Subject: [PATCH 1/5] chore: add editorconfig Signed-off-by: Mark Sagi-Kazar --- .editorconfig | 15 +++++++++++++++ .github/workflows/.editorconfig | 2 +- scripts/manifests/.editorconfig | 2 ++ 3 files changed, 18 insertions(+), 1 deletion(-) create mode 100644 .editorconfig create mode 100644 scripts/manifests/.editorconfig diff --git a/.editorconfig b/.editorconfig new file mode 100644 index 00000000..6ccdbfc9 --- /dev/null +++ b/.editorconfig @@ -0,0 +1,15 @@ +root = true + +[*] +charset = utf-8 +end_of_line = lf +indent_size = 4 +indent_style = space +insert_final_newline = true +trim_trailing_whitespace = true + +[*.go] +indent_style = tab + +[*.proto] +indent_size = 2 diff --git a/.github/workflows/.editorconfig b/.github/workflows/.editorconfig index 7bd3346f..0902c6ae 100644 --- a/.github/workflows/.editorconfig +++ b/.github/workflows/.editorconfig @@ -1,2 +1,2 @@ -[*.yml] +[{*.yml,*.yaml}] indent_size = 2 diff --git a/scripts/manifests/.editorconfig b/scripts/manifests/.editorconfig new file mode 100644 index 00000000..0902c6ae --- /dev/null +++ b/scripts/manifests/.editorconfig @@ -0,0 +1,2 @@ +[{*.yml,*.yaml}] +indent_size = 2 From 3841f05ba43a6d31dac623fd11e6258422235089 Mon Sep 17 00:00:00 2001 From: Mark Sagi-Kazar Date: Tue, 3 Nov 2020 20:43:59 +0100 Subject: [PATCH 2/5] Update linter config Signed-off-by: Mark Sagi-Kazar --- .golangci.yml | 112 ++++++++++++++++++++++++++++++++------------------ 1 file changed, 73 insertions(+), 39 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index ac9134bf..77ca3290 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,45 +1,79 @@ run: - timeout: 2m + timeout: 2m linters-settings: - golint: - min-confidence: 0.1 - goimports: - local-prefixes: github.com/dexidp/dex + gci: + local-prefixes: github.com/dexidp/dex + goimports: + local-prefixes: github.com/dexidp/dex + golint: + min-confidence: 0 + linters: - disable-all: true - enable: - - bodyclose - - deadcode - - depguard - - dogsled - - gochecknoinits - - gofmt - - goimports - - golint - - gosimple - - gocritic - - govet - - ineffassign - - interfacer - - misspell - - nakedret - - staticcheck - - structcheck - - stylecheck - - typecheck - - unconvert - - unused - - varcheck - - whitespace + disable-all: true + enable: + - bodyclose + - deadcode + - dogsled + - exhaustive + - exportloopref + - gochecknoinits + - gocritic + - gofmt + - goimports + - golint + - goprintffuncname + - gosimple + - govet + - ineffassign + - misspell + - nakedret + - nolintlint + - rowserrcheck + - staticcheck + - structcheck + - stylecheck + - typecheck + - unconvert + - unused + - varcheck + - whitespace - # TODO: fix linter errors before enabling - # - unparam - # - scopelint - # - gosec - # - gocyclo - # - lll - # - goconst - # - errcheck - # - dupl + # TODO: fix linter errors before enabling + # - gci + # - gochecknoglobals + # - gocognit + # - godot + # - gofumpt + # - nlreturn + # - noctx + # - prealloc + # - sqlclosecheck + + # TODO: fix linter errors before enabling (from original config) + # - dupl + # - errcheck + # - goconst + # - gocyclo + # - gosec + # - lll + # - scopelint + # - unparam + + # unused + # - depguard + # - goheader + # - gomodguard + + # don't enable: + # - asciicheck + # - funlen + # - godox + # - goerr113 + # - gomnd + # - interfacer + # - maligned + # - nestif + # - testpackage + # - wsl From cafea292cab275ea3d50b0a9ad8ccb33ee245980 Mon Sep 17 00:00:00 2001 From: Mark Sagi-Kazar Date: Tue, 3 Nov 2020 20:47:43 +0100 Subject: [PATCH 3/5] Update linter Signed-off-by: Mark Sagi-Kazar --- .golangci.yml | 4 ++++ Makefile | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/.golangci.yml b/.golangci.yml index 77ca3290..16514f7e 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -34,6 +34,7 @@ linters: - staticcheck - structcheck - stylecheck + - tparallel - typecheck - unconvert - unused @@ -41,8 +42,10 @@ linters: - whitespace # TODO: fix linter errors before enabling + # - exhaustivestruct # - gci # - gochecknoglobals + # - errorlint # - gocognit # - godot # - gofumpt @@ -50,6 +53,7 @@ linters: # - noctx # - prealloc # - sqlclosecheck + # - wrapcheck # TODO: fix linter errors before enabling (from original config) # - dupl diff --git a/Makefile b/Makefile index b665bcfc..65042522 100644 --- a/Makefile +++ b/Makefile @@ -18,7 +18,7 @@ export GOBIN=$(PWD)/bin LD_FLAGS="-w -X $(REPO_PATH)/version.Version=$(VERSION)" # Dependency versions -GOLANGCI_VERSION = 1.31.0 +GOLANGCI_VERSION = 1.32.2 build: bin/dex From 84ea79088501b824b9f2990fb563ef972154836a Mon Sep 17 00:00:00 2001 From: Mark Sagi-Kazar Date: Tue, 3 Nov 2020 20:49:29 +0100 Subject: [PATCH 4/5] Enable gci and gofumpt Signed-off-by: Mark Sagi-Kazar --- .golangci.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 16514f7e..fa8f5045 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -18,9 +18,11 @@ linters: - dogsled - exhaustive - exportloopref + - gci - gochecknoinits - gocritic - gofmt + - gofumpt - goimports - golint - goprintffuncname @@ -43,12 +45,10 @@ linters: # TODO: fix linter errors before enabling # - exhaustivestruct - # - gci # - gochecknoglobals # - errorlint # - gocognit # - godot - # - gofumpt # - nlreturn # - noctx # - prealloc From 349832b380b9d9325194fbb56bacd87f64d3b48f Mon Sep 17 00:00:00 2001 From: Mark Sagi-Kazar Date: Tue, 3 Nov 2020 20:50:09 +0100 Subject: [PATCH 5/5] Run fixer Signed-off-by: Mark Sagi-Kazar --- cmd/dex/config_test.go | 1 + connector/github/github.go | 7 +++-- connector/keystone/keystone.go | 7 +++-- connector/keystone/keystone_test.go | 44 +++++++++++++++++---------- connector/openshift/openshift.go | 11 ++----- connector/openshift/openshift_test.go | 7 ++--- server/api.go | 1 - storage/conformance/conformance.go | 6 ++-- storage/etcd/config.go | 4 +-- storage/sql/config_test.go | 8 +++-- storage/sql/crud.go | 1 - storage/sql/migrate.go | 27 ++++++++++------ storage/sql/postgres_test.go | 3 +- 13 files changed, 71 insertions(+), 56 deletions(-) diff --git a/cmd/dex/config_test.go b/cmd/dex/config_test.go index bced93b5..278a9115 100644 --- a/cmd/dex/config_test.go +++ b/cmd/dex/config_test.go @@ -59,6 +59,7 @@ func TestInvalidConfiguration(t *testing.T) { t.Fatalf("Expected error message to be %q, got %q", wanted, got) } } + func TestUnmarshalConfig(t *testing.T) { rawConfig := []byte(` issuer: http://127.0.0.1:5556/dex diff --git a/connector/github/github.go b/connector/github/github.go index 6b95ea8f..02f2cae8 100644 --- a/connector/github/github.go +++ b/connector/github/github.go @@ -35,8 +35,10 @@ const ( // Pagination URL patterns // https://developer.github.com/v3/#pagination -var reNext = regexp.MustCompile("<([^>]+)>; rel=\"next\"") -var reLast = regexp.MustCompile("<([^>]+)>; rel=\"last\"") +var ( + reNext = regexp.MustCompile("<([^>]+)>; rel=\"next\"") + reLast = regexp.MustCompile("<([^>]+)>; rel=\"last\"") +) // Config holds configuration options for github logins. type Config struct { @@ -626,7 +628,6 @@ func (c *githubConnector) userInOrg(ctx context.Context, client *http.Client, us apiURL := fmt.Sprintf("%s/orgs/%s/members/%s", c.apiURL, orgName, userName) req, err := http.NewRequest("GET", apiURL, nil) - if err != nil { return false, fmt.Errorf("github: new req: %v", err) } diff --git a/connector/keystone/keystone.go b/connector/keystone/keystone.go index 92682f78..d817bd94 100644 --- a/connector/keystone/keystone.go +++ b/connector/keystone/keystone.go @@ -115,7 +115,8 @@ func (c *Config) Open(id string, logger log.Logger) (connector.Connector, error) c.Host, c.AdminUsername, c.AdminPassword, - logger}, nil + logger, + }, nil } func (p *conn) Close() error { return nil } @@ -137,7 +138,7 @@ func (p *conn) Login(ctx context.Context, scopes connector.Scopes, username, pas return identity, false, err } defer resp.Body.Close() - var tokenResp = new(tokenResponse) + tokenResp := new(tokenResponse) err = json.Unmarshal(data, &tokenResp) if err != nil { return identity, false, fmt.Errorf("keystone: invalid token response: %v", err) @@ -295,7 +296,7 @@ func (p *conn) getUserGroups(ctx context.Context, userID string, token string) ( } defer resp.Body.Close() - var groupsResp = new(groupsResponse) + groupsResp := new(groupsResponse) err = json.Unmarshal(data, &groupsResp) if err != nil { diff --git a/connector/keystone/keystone_test.go b/connector/keystone/keystone_test.go index d66da8be..b1380124 100644 --- a/connector/keystone/keystone_test.go +++ b/connector/keystone/keystone_test.go @@ -84,7 +84,7 @@ func getAdminToken(t *testing.T, adminName, adminPass string) (token, id string) } defer resp.Body.Close() - var tokenResp = new(tokenResponse) + tokenResp := new(tokenResponse) err = json.Unmarshal(data, &tokenResp) if err != nil { t.Fatal(err) @@ -128,7 +128,7 @@ func createUser(t *testing.T, token, userName, userEmail, userPass string) strin } defer resp.Body.Close() - var userResp = new(userResponse) + userResp := new(userResponse) err = json.Unmarshal(data, &userResp) if err != nil { t.Fatal(err) @@ -189,7 +189,7 @@ func createGroup(t *testing.T, token, description, name string) string { } defer resp.Body.Close() - var groupResp = new(groupResponse) + groupResp := new(groupResponse) err = json.Unmarshal(data, &groupResp) if err != nil { t.Fatal(err) @@ -219,8 +219,10 @@ func addUserToGroup(t *testing.T, token, groupID, userID string) error { func TestIncorrectCredentialsLogin(t *testing.T) { setupVariables(t) - c := conn{Host: keystoneURL, Domain: testDomain, - AdminUsername: adminUser, AdminPassword: adminPass} + c := conn{ + Host: keystoneURL, Domain: testDomain, + AdminUsername: adminUser, AdminPassword: adminPass, + } s := connector.Scopes{OfflineAccess: true, Groups: true} _, validPW, err := c.Login(context.Background(), s, adminUser, invalidPass) @@ -254,7 +256,7 @@ func TestValidUserLogin(t *testing.T) { verifiedEmail bool } - var tests = []struct { + tests := []struct { name string input tUser expected expect @@ -294,8 +296,10 @@ func TestValidUserLogin(t *testing.T) { userID := createUser(t, token, tt.input.username, tt.input.email, tt.input.password) defer deleteResource(t, token, userID, usersURL) - c := conn{Host: keystoneURL, Domain: tt.input.domain, - AdminUsername: adminUser, AdminPassword: adminPass} + c := conn{ + Host: keystoneURL, Domain: tt.input.domain, + AdminUsername: adminUser, AdminPassword: adminPass, + } s := connector.Scopes{OfflineAccess: true, Groups: true} identity, validPW, err := c.Login(context.Background(), s, tt.input.username, tt.input.password) if err != nil { @@ -329,8 +333,10 @@ func TestUseRefreshToken(t *testing.T) { addUserToGroup(t, token, groupID, adminID) defer deleteResource(t, token, groupID, groupsURL) - c := conn{Host: keystoneURL, Domain: testDomain, - AdminUsername: adminUser, AdminPassword: adminPass} + c := conn{ + Host: keystoneURL, Domain: testDomain, + AdminUsername: adminUser, AdminPassword: adminPass, + } s := connector.Scopes{OfflineAccess: true, Groups: true} identityLogin, _, err := c.Login(context.Background(), s, adminUser, adminPass) @@ -352,8 +358,10 @@ func TestUseRefreshTokenUserDeleted(t *testing.T) { token, _ := getAdminToken(t, adminUser, adminPass) userID := createUser(t, token, testUser, testEmail, testPass) - c := conn{Host: keystoneURL, Domain: testDomain, - AdminUsername: adminUser, AdminPassword: adminPass} + c := conn{ + Host: keystoneURL, Domain: testDomain, + AdminUsername: adminUser, AdminPassword: adminPass, + } s := connector.Scopes{OfflineAccess: true, Groups: true} identityLogin, _, err := c.Login(context.Background(), s, testUser, testPass) @@ -380,8 +388,10 @@ func TestUseRefreshTokenGroupsChanged(t *testing.T) { userID := createUser(t, token, testUser, testEmail, testPass) defer deleteResource(t, token, userID, usersURL) - c := conn{Host: keystoneURL, Domain: testDomain, - AdminUsername: adminUser, AdminPassword: adminPass} + c := conn{ + Host: keystoneURL, Domain: testDomain, + AdminUsername: adminUser, AdminPassword: adminPass, + } s := connector.Scopes{OfflineAccess: true, Groups: true} identityLogin, _, err := c.Login(context.Background(), s, testUser, testPass) @@ -414,8 +424,10 @@ func TestNoGroupsInScope(t *testing.T) { userID := createUser(t, token, testUser, testEmail, testPass) defer deleteResource(t, token, userID, usersURL) - c := conn{Host: keystoneURL, Domain: testDomain, - AdminUsername: adminUser, AdminPassword: adminPass} + c := conn{ + Host: keystoneURL, Domain: testDomain, + AdminUsername: adminUser, AdminPassword: adminPass, + } s := connector.Scopes{OfflineAccess: true, Groups: false} groupID := createGroup(t, token, "Test group", testGroup) diff --git a/connector/openshift/openshift.go b/connector/openshift/openshift.go index 06669356..f06e8f80 100644 --- a/connector/openshift/openshift.go +++ b/connector/openshift/openshift.go @@ -12,13 +12,12 @@ import ( "strings" "time" + "golang.org/x/oauth2" + "github.com/dexidp/dex/connector" "github.com/dexidp/dex/pkg/groups" "github.com/dexidp/dex/pkg/log" - "github.com/dexidp/dex/storage/kubernetes/k8sapi" - - "golang.org/x/oauth2" ) // Config holds configuration options for OpenShift login @@ -32,9 +31,7 @@ type Config struct { RootCA string `json:"rootCA"` } -var ( - _ connector.CallbackConnector = (*openshiftConnector)(nil) -) +var _ connector.CallbackConnector = (*openshiftConnector)(nil) type openshiftConnector struct { apiURL string @@ -89,7 +86,6 @@ func (c *Config) Open(id string, logger log.Logger) (conn connector.Connector, e } resp, err := openshiftConnector.httpClient.Do(req.WithContext(ctx)) - if err != nil { cancel() return nil, fmt.Errorf("failed to query OpenShift endpoint %v", err) @@ -160,7 +156,6 @@ func (c *openshiftConnector) HandleCallback(s connector.Scopes, r *http.Request) client := c.oauth2Config.Client(ctx, token) user, err := c.user(ctx, client) - if err != nil { return identity, fmt.Errorf("openshift: get user: %v", err) } diff --git a/connector/openshift/openshift_test.go b/connector/openshift/openshift_test.go index d4efa14d..90f1686c 100644 --- a/connector/openshift/openshift_test.go +++ b/connector/openshift/openshift_test.go @@ -10,12 +10,11 @@ import ( "reflect" "testing" - "github.com/dexidp/dex/connector" - - "github.com/dexidp/dex/storage/kubernetes/k8sapi" - "github.com/sirupsen/logrus" "golang.org/x/oauth2" + + "github.com/dexidp/dex/connector" + "github.com/dexidp/dex/storage/kubernetes/k8sapi" ) func TestOpen(t *testing.T) { diff --git a/server/api.go b/server/api.go index c815ada4..98fd20b8 100644 --- a/server/api.go +++ b/server/api.go @@ -96,7 +96,6 @@ func (d dexAPI) UpdateClient(ctx context.Context, req *api.UpdateClientReq) (*ap } return old, nil }) - if err != nil { if err == storage.ErrNotFound { return &api.UpdateClientResp{NotFound: true}, nil diff --git a/storage/conformance/conformance.go b/storage/conformance/conformance.go index dd2083ae..92611cc1 100644 --- a/storage/conformance/conformance.go +++ b/storage/conformance/conformance.go @@ -7,13 +7,11 @@ import ( "testing" "time" + "github.com/kylelemons/godebug/pretty" + "golang.org/x/crypto/bcrypt" jose "gopkg.in/square/go-jose.v2" - "golang.org/x/crypto/bcrypt" - "github.com/dexidp/dex/storage" - - "github.com/kylelemons/godebug/pretty" ) // ensure that values being tested on never expire. diff --git a/storage/etcd/config.go b/storage/etcd/config.go index e294a53a..14607316 100644 --- a/storage/etcd/config.go +++ b/storage/etcd/config.go @@ -11,9 +11,7 @@ import ( "github.com/dexidp/dex/storage" ) -var ( - defaultDialTimeout = 2 * time.Second -) +var defaultDialTimeout = 2 * time.Second // SSL represents SSL options for etcd databases. type SSL struct { diff --git a/storage/sql/config_test.go b/storage/sql/config_test.go index 4df32cf4..2085bc88 100644 --- a/storage/sql/config_test.go +++ b/storage/sql/config_test.go @@ -34,8 +34,10 @@ func withTimeout(t time.Duration, f func()) { } func cleanDB(c *conn) error { - tables := []string{"client", "auth_request", "auth_code", - "refresh_token", "keys", "password"} + tables := []string{ + "client", "auth_request", "auth_code", + "refresh_token", "keys", "password", + } for _, tbl := range tables { _, err := c.Exec("delete from " + tbl) @@ -97,7 +99,7 @@ func getenv(key, defaultVal string) string { const testPostgresEnv = "DEX_POSTGRES_HOST" func TestCreateDataSourceName(t *testing.T) { - var testCases = []struct { + testCases := []struct { description string input *Postgres expected string diff --git a/storage/sql/crud.go b/storage/sql/crud.go index dedfd2a8..c804fc43 100644 --- a/storage/sql/crud.go +++ b/storage/sql/crud.go @@ -244,7 +244,6 @@ func (c *conn) CreateAuthCode(a storage.AuthCode) error { encoder(a.Claims.Groups), a.ConnectorID, a.ConnectorData, a.Expiry, a.PKCE.CodeChallenge, a.PKCE.CodeChallengeMethod, ) - if err != nil { if c.alreadyExistsCheck(err) { return storage.ErrAlreadyExists diff --git a/storage/sql/migrate.go b/storage/sql/migrate.go index 8201d443..460658c2 100644 --- a/storage/sql/migrate.go +++ b/storage/sql/migrate.go @@ -82,7 +82,8 @@ type migration struct { // All SQL flavors share migration strategies. var migrations = []migration{ { - stmts: []string{` + stmts: []string{ + ` create table client ( id text not null primary key, secret text not null, @@ -170,7 +171,8 @@ var migrations = []migration{ }, }, { - stmts: []string{` + stmts: []string{ + ` alter table refresh_token add column token text not null default '';`, ` @@ -182,7 +184,8 @@ var migrations = []migration{ }, }, { - stmts: []string{` + stmts: []string{ + ` create table offline_session ( user_id text not null, conn_id text not null, @@ -192,7 +195,8 @@ var migrations = []migration{ }, }, { - stmts: []string{` + stmts: []string{ + ` create table connector ( id text not null primary key, type text not null, @@ -203,7 +207,8 @@ var migrations = []migration{ }, }, { - stmts: []string{` + stmts: []string{ + ` alter table auth_code add column claims_preferred_username text not null default '';`, ` @@ -215,14 +220,16 @@ var migrations = []migration{ }, }, { - stmts: []string{` + stmts: []string{ + ` alter table offline_session add column connector_data bytea; `, }, }, { - stmts: []string{` + stmts: []string{ + ` alter table auth_request modify column state varchar(4096); `, @@ -230,7 +237,8 @@ var migrations = []migration{ flavor: &flavorMySQL, }, { - stmts: []string{` + stmts: []string{ + ` create table device_request ( user_code text not null primary key, device_code text not null, @@ -251,7 +259,8 @@ var migrations = []migration{ }, }, { - stmts: []string{` + stmts: []string{ + ` alter table auth_request add column code_challenge text not null default '';`, ` diff --git a/storage/sql/postgres_test.go b/storage/sql/postgres_test.go index 8071dc29..d58abc70 100644 --- a/storage/sql/postgres_test.go +++ b/storage/sql/postgres_test.go @@ -34,7 +34,8 @@ func TestPostgresTunables(t *testing.T) { }, SSL: SSL{ Mode: pgSSLDisable, // Postgres container doesn't support SSL. - }} + }, + } t.Run("with nothing set, uses defaults", func(t *testing.T) { cfg := *baseCfg