From 72d1ecab64d7936bba6a0d6f0dd5a8b95216c4cd Mon Sep 17 00:00:00 2001 From: Eric Chiang Date: Tue, 9 Feb 2016 12:57:42 -0800 Subject: [PATCH] *: remove in memory password info repo --- admin/api_test.go | 19 ++++-- db/password.go | 10 +++ functional/repo/password_repo_test.go | 11 ++-- integration/common_test.go | 8 ++- integration/oidc_test.go | 2 +- server/config.go | 2 +- server/testutil.go | 5 +- test | 2 +- user/api/api_test.go | 27 +++++--- user/email/email_test.go | 29 +++++---- user/manager/manager_test.go | 31 +++++++--- user/password.go | 88 --------------------------- user/password_test.go | 43 ------------- 13 files changed, 100 insertions(+), 177 deletions(-) diff --git a/admin/api_test.go b/admin/api_test.go index 5a4ba827..daaf913e 100644 --- a/admin/api_test.go +++ b/admin/api_test.go @@ -46,12 +46,19 @@ func makeTestFixtures() *testFixtures { return repo }() - f.pwr = user.NewPasswordInfoRepoFromPasswordInfos([]user.PasswordInfo{ - { - UserID: "ID-1", - Password: []byte("hi."), - }, - }) + f.pwr = func() user.PasswordInfoRepo { + repo, err := db.NewPasswordInfoRepoFromPasswordInfos(dbMap, []user.PasswordInfo{ + { + UserID: "ID-1", + Password: []byte("hi."), + }, + }) + if err != nil { + panic("Failed to create user repo: " + err.Error()) + } + return repo + }() + ccr := connector.NewConnectorConfigRepoFromConfigs([]connector.ConnectorConfig{ &connector.LocalConnectorConfig{ID: "local"}, }) diff --git a/db/password.go b/db/password.go index be278c2c..afd2f247 100644 --- a/db/password.go +++ b/db/password.go @@ -38,6 +38,16 @@ func NewPasswordInfoRepo(dbm *gorp.DbMap) user.PasswordInfoRepo { } } +func NewPasswordInfoRepoFromPasswordInfos(dbm *gorp.DbMap, infos []user.PasswordInfo) (user.PasswordInfoRepo, error) { + repo := NewPasswordInfoRepo(dbm) + for _, info := range infos { + if err := repo.Create(nil, info); err != nil { + return nil, err + } + } + return repo, nil +} + type passwordInfoRepo struct { dbMap *gorp.DbMap } diff --git a/functional/repo/password_repo_test.go b/functional/repo/password_repo_test.go index de82326c..8bc6eefe 100644 --- a/functional/repo/password_repo_test.go +++ b/functional/repo/password_repo_test.go @@ -5,6 +5,7 @@ import ( "testing" "time" + "github.com/go-gorp/gorp" "github.com/kylelemons/godebug/pretty" "github.com/coreos/dex/db" @@ -21,12 +22,14 @@ var ( ) func newPasswordInfoRepo(t *testing.T) user.PasswordInfoRepo { + var dbMap *gorp.DbMap if os.Getenv("DEX_TEST_DSN") == "" { - return user.NewPasswordInfoRepoFromPasswordInfos(testPWs) + dbMap = db.NewMemDB() + } else { + dbMap = connect(t) } - dbMap := connect(t) - repo := db.NewPasswordInfoRepo(dbMap) - if err := user.LoadPasswordInfos(repo, testPWs); err != nil { + repo, err := db.NewPasswordInfoRepoFromPasswordInfos(dbMap, testPWs) + if err != nil { t.Fatalf("Unable to add password infos: %v", err) } return repo diff --git a/integration/common_test.go b/integration/common_test.go index 058f7968..fc8b0d04 100644 --- a/integration/common_test.go +++ b/integration/common_test.go @@ -53,7 +53,13 @@ func makeUserObjects(users []user.UserWithRemoteIdentities, passwords []user.Pas } return repo }() - pwr := user.NewPasswordInfoRepoFromPasswordInfos(passwords) + pwr := func() user.PasswordInfoRepo { + repo, err := db.NewPasswordInfoRepoFromPasswordInfos(dbMap, passwords) + if err != nil { + panic("Failed to create password info repo: " + err.Error()) + } + return repo + }() ccr := connector.NewConnectorConfigRepoFromConfigs( []connector.ConnectorConfig{&connector.LocalConnectorConfig{ID: "local"}}, diff --git a/integration/oidc_test.go b/integration/oidc_test.go index d4d32f62..cacdd526 100644 --- a/integration/oidc_test.go +++ b/integration/oidc_test.go @@ -144,7 +144,7 @@ func TestHTTPExchangeTokenRefreshToken(t *testing.T) { t.Fatalf("Unexpected error: %v", err) } - passwordInfoRepo := user.NewPasswordInfoRepo() + passwordInfoRepo := db.NewPasswordInfoRepo(db.NewMemDB()) refreshTokenRepo := refreshtest.NewTestRefreshTokenRepo() srv := &server.Server{ diff --git a/server/config.go b/server/config.go index 2e4ae35b..83d88a02 100644 --- a/server/config.go +++ b/server/config.go @@ -142,7 +142,7 @@ func (cfg *SingleServerConfig) Configure(srv *Server) error { return err } - pwiRepo := user.NewPasswordInfoRepo() + pwiRepo := db.NewPasswordInfoRepo(dbMap) refTokRepo := db.NewRefreshTokenRepo(dbMap) diff --git a/server/testutil.go b/server/testutil.go index 579fdf04..e32ff1aa 100644 --- a/server/testutil.go +++ b/server/testutil.go @@ -95,7 +95,10 @@ func makeTestFixtures() (*testFixtures, error) { if err != nil { return nil, err } - pwRepo := user.NewPasswordInfoRepoFromPasswordInfos(testPasswordInfos) + pwRepo, err := db.NewPasswordInfoRepoFromPasswordInfos(dbMap, testPasswordInfos) + if err != nil { + return nil, err + } connConfigs := []connector.ConnectorConfig{ &connector.OIDCConnectorConfig{ diff --git a/test b/test index 0ea61d78..221f23ba 100755 --- a/test +++ b/test @@ -14,7 +14,7 @@ COVER=${COVER:-"-cover"} source ./build -TESTABLE="connector db integration pkg/crypto pkg/flag pkg/http pkg/net pkg/time pkg/html functional/repo server session session/manager user user/api user/manager email admin" +TESTABLE="connector db integration pkg/crypto pkg/flag pkg/http pkg/net pkg/time pkg/html functional/repo server session session/manager user user/api user/manager user/email email admin" FORMATTABLE="$TESTABLE cmd/dexctl cmd/dex-worker cmd/dex-overlord examples/app functional pkg/log" # user has not provided PKG override diff --git a/user/api/api_test.go b/user/api/api_test.go index d85aade2..ed121387 100644 --- a/user/api/api_test.go +++ b/user/api/api_test.go @@ -123,16 +123,23 @@ func makeTestFixtures() (*UsersAPI, *testEmailer) { return repo }() - pwr := user.NewPasswordInfoRepoFromPasswordInfos([]user.PasswordInfo{ - { - UserID: "ID-1", - Password: []byte("password-1"), - }, - { - UserID: "ID-2", - Password: []byte("password-2"), - }, - }) + pwr := func() user.PasswordInfoRepo { + repo, err := db.NewPasswordInfoRepoFromPasswordInfos(dbMap, []user.PasswordInfo{ + { + UserID: "ID-1", + Password: []byte("password-1"), + }, + { + UserID: "ID-2", + Password: []byte("password-2"), + }, + }) + if err != nil { + panic("Failed to create user repo: " + err.Error()) + } + return repo + }() + ccr := connector.NewConnectorConfigRepoFromConfigs([]connector.ConnectorConfig{ &connector.LocalConnectorConfig{ID: "local"}, }) diff --git a/user/email/email_test.go b/user/email/email_test.go index eb40642f..453025ea 100644 --- a/user/email/email_test.go +++ b/user/email/email_test.go @@ -46,8 +46,9 @@ func (t *testEmailer) SendMail(from, subject, text, html string, to ...string) e } func makeTestFixtures() (*UserEmailer, *testEmailer, *key.PublicKey) { + dbMap := db.NewMemDB() ur := func() user.UserRepo { - repo, err := db.NewUserRepoFromUsers(db.NewMemDB(), []user.UserWithRemoteIdentities{ + repo, err := db.NewUserRepoFromUsers(dbMap, []user.UserWithRemoteIdentities{ { User: user.User{ ID: "ID-1", @@ -72,16 +73,22 @@ func makeTestFixtures() (*UserEmailer, *testEmailer, *key.PublicKey) { return repo }() - pwr := user.NewPasswordInfoRepoFromPasswordInfos([]user.PasswordInfo{ - { - UserID: "ID-1", - Password: []byte("password-1"), - }, - { - UserID: "ID-2", - Password: []byte("password-2"), - }, - }) + pwr := func() user.PasswordInfoRepo { + repo, err := db.NewPasswordInfoRepoFromPasswordInfos(dbMap, []user.PasswordInfo{ + { + UserID: "ID-1", + Password: []byte("password-1"), + }, + { + UserID: "ID-2", + Password: []byte("password-2"), + }, + }) + if err != nil { + panic("Failed to create user repo: " + err.Error()) + } + return repo + }() privKey, err := key.GeneratePrivateKey() if err != nil { diff --git a/user/manager/manager_test.go b/user/manager/manager_test.go index 337f790e..4599c572 100644 --- a/user/manager/manager_test.go +++ b/user/manager/manager_test.go @@ -60,16 +60,23 @@ func makeTestFixtures() *testFixtures { return repo }() - f.pwr = user.NewPasswordInfoRepoFromPasswordInfos([]user.PasswordInfo{ - { - UserID: "ID-1", - Password: []byte("password-1"), - }, - { - UserID: "ID-2", - Password: []byte("password-2"), - }, - }) + f.pwr = func() user.PasswordInfoRepo { + repo, err := db.NewPasswordInfoRepoFromPasswordInfos(dbMap, []user.PasswordInfo{ + { + UserID: "ID-1", + Password: []byte("password-1"), + }, + { + UserID: "ID-2", + Password: []byte("password-2"), + }, + }) + if err != nil { + panic("Failed to create user repo: " + err.Error()) + } + return repo + }() + f.ccr = connector.NewConnectorConfigRepoFromConfigs([]connector.ConnectorConfig{ &connector.LocalConnectorConfig{ID: "local"}, }) @@ -215,18 +222,22 @@ func TestRegisterWithPassword(t *testing.T) { } if diff := pretty.Compare(usr, ridUSR); diff != "" { t.Errorf("case %d: Compare(want, got) = %v", i, diff) + continue } pwi, err := f.pwr.Get(nil, userID) if err != nil { t.Errorf("case %d: err != nil: %q", i, err) + continue } ident, err := pwi.Authenticate(tt.plaintext) if err != nil { t.Errorf("case %d: err != nil: %q", i, err) + continue } if ident.ID != userID { t.Errorf("case %d: ident.ID: want=%q, got=%q", i, userID, ident.ID) + continue } _, err = pwi.Authenticate(tt.plaintext + "WRONG") diff --git a/user/password.go b/user/password.go index 4b93f349..6099d1ca 100644 --- a/user/password.go +++ b/user/password.go @@ -4,9 +4,7 @@ import ( "encoding/json" "errors" "fmt" - "io" "net/url" - "os" "time" "golang.org/x/crypto/bcrypt" @@ -85,60 +83,6 @@ type PasswordInfoRepo interface { Create(repo.Transaction, PasswordInfo) error } -func NewPasswordInfoRepo() PasswordInfoRepo { - return &memPasswordInfoRepo{ - pws: make(map[string]PasswordInfo), - } -} - -type memPasswordInfoRepo struct { - pws map[string]PasswordInfo -} - -func (m *memPasswordInfoRepo) Get(_ repo.Transaction, id string) (PasswordInfo, error) { - pw, ok := m.pws[id] - if !ok { - return PasswordInfo{}, ErrorNotFound - } - return pw, nil -} - -func (m *memPasswordInfoRepo) Create(_ repo.Transaction, pw PasswordInfo) error { - _, ok := m.pws[pw.UserID] - if ok { - return ErrorDuplicateID - } - - if pw.UserID == "" { - return ErrorInvalidID - } - - if len(pw.Password) == 0 { - return ErrorInvalidPassword - } - - m.pws[pw.UserID] = pw - return nil -} - -func (m *memPasswordInfoRepo) Update(_ repo.Transaction, pw PasswordInfo) error { - if pw.UserID == "" { - return ErrorInvalidID - } - - _, ok := m.pws[pw.UserID] - if !ok { - return ErrorNotFound - } - - if len(pw.Password) == 0 { - return ErrorInvalidPassword - } - - m.pws[pw.UserID] = pw - return nil -} - func (u *PasswordInfo) UnmarshalJSON(data []byte) error { var dec struct { UserID string `json:"userId"` @@ -172,21 +116,6 @@ func (u *PasswordInfo) UnmarshalJSON(data []byte) error { return nil } -func newPasswordInfosFromReader(r io.Reader) ([]PasswordInfo, error) { - var pws []PasswordInfo - err := json.NewDecoder(r).Decode(&pws) - return pws, err -} - -func readPasswordInfosFromFile(loc string) ([]PasswordInfo, error) { - pwf, err := os.Open(loc) - if err != nil { - return nil, fmt.Errorf("unable to read password info from file %q: %v", loc, err) - } - - return newPasswordInfosFromReader(pwf) -} - func LoadPasswordInfos(repo PasswordInfoRepo, pws []PasswordInfo) error { for i, pw := range pws { err := repo.Create(nil, pw) @@ -197,23 +126,6 @@ func LoadPasswordInfos(repo PasswordInfoRepo, pws []PasswordInfo) error { return nil } -func NewPasswordInfoRepoFromPasswordInfos(pws []PasswordInfo) PasswordInfoRepo { - memRepo := NewPasswordInfoRepo().(*memPasswordInfoRepo) - for _, pw := range pws { - memRepo.pws[pw.UserID] = pw - } - return memRepo -} - -func NewPasswordInfoRepoFromFile(loc string) (PasswordInfoRepo, error) { - pws, err := readPasswordInfosFromFile(loc) - if err != nil { - return nil, err - } - - return NewPasswordInfoRepoFromPasswordInfos(pws), nil -} - func NewPasswordReset(userID string, password Password, issuer url.URL, clientID string, callback url.URL, expires time.Duration) PasswordReset { claims := oidc.NewClaims(issuer.String(), userID, clientID, clock.Now(), clock.Now().Add(expires)) claims.Add(ClaimPasswordResetPassword, string(password)) diff --git a/user/password_test.go b/user/password_test.go index 9fd17cf4..1625ece7 100644 --- a/user/password_test.go +++ b/user/password_test.go @@ -2,7 +2,6 @@ package user import ( "net/url" - "strings" "testing" "time" @@ -14,48 +13,6 @@ import ( "github.com/coreos/go-oidc/key" ) -func TestNewPasswordInfosFromReader(t *testing.T) { - PasswordHasher = func(plaintext string) ([]byte, error) { - return []byte(strings.ToUpper(plaintext)), nil - } - defer func() { - PasswordHasher = DefaultPasswordHasher - }() - - tests := []struct { - json string - want []PasswordInfo - }{ - { - json: `[{"userId":"12345","passwordPlaintext":"password"},{"userId":"78901","passwordHash":"WORDPASS", "passwordExpires":"2006-01-01T15:04:05Z"}]`, - want: []PasswordInfo{ - { - UserID: "12345", - Password: []byte("PASSWORD"), - }, - { - UserID: "78901", - Password: []byte("WORDPASS"), - PasswordExpires: time.Date(2006, - 1, 1, 15, 4, 5, 0, time.UTC), - }, - }, - }, - } - - for i, tt := range tests { - r := strings.NewReader(tt.json) - us, err := newPasswordInfosFromReader(r) - if err != nil { - t.Errorf("case %d: want nil err: %v", i, err) - continue - } - if diff := pretty.Compare(tt.want, us); diff != "" { - t.Errorf("case %d: Compare(want, got): %v", i, diff) - } - } -} - func TestNewPasswordFromHash(t *testing.T) { tests := []string{ "test",