From ac73d3cdf2b7765c7b8b06e9f7e880dc583a3ffd Mon Sep 17 00:00:00 2001 From: Eric Chiang Date: Mon, 4 Apr 2016 18:05:00 -0700 Subject: [PATCH] *: load password infos from users file in no-db mode not connectors In --no-db mode, load passwords from the users file instead of the connectors file. This allows us to remove the password infos field from the local connector and stop loading them during connector registration, a case that was causing panics when using a real database (see #286). Fixes #286 Closes #340 --- connector/config_repo_test.go | 20 ------- connector/connector_local.go | 3 +- integration/oidc_test.go | 7 ++- server/config.go | 49 ++++++++++++++-- server/config_test.go | 78 ++++++++++++++++++++++++++ server/server.go | 13 ----- static/fixtures/connectors.json.sample | 12 +--- static/fixtures/users.json.sample | 9 ++- 8 files changed, 132 insertions(+), 59 deletions(-) create mode 100644 server/config_test.go diff --git a/connector/config_repo_test.go b/connector/config_repo_test.go index 6bb98c13..46ed7a40 100644 --- a/connector/config_repo_test.go +++ b/connector/config_repo_test.go @@ -60,23 +60,9 @@ func TestNewConnectorConfigFromMap(t *testing.T) { m: map[string]interface{}{ "type": "local", "id": "foo", - "passwordInfos": []map[string]string{ - {"userId": "abc", "passwordHash": "UElORw=="}, // []byte is base64 encoded when using json.Marshasl - {"userId": "271", "passwordPlaintext": "pong"}, - }, }, want: &LocalConnectorConfig{ ID: "foo", - PasswordInfos: []user.PasswordInfo{ - user.PasswordInfo{ - UserID: "abc", - Password: user.Password("PING"), - }, - user.PasswordInfo{ - UserID: "271", - Password: user.Password("PONG"), - }, - }, }, }, { @@ -111,12 +97,6 @@ func TestNewConnectorConfigFromMap(t *testing.T) { func TestNewConnectorConfigFromMapFail(t *testing.T) { tests := []map[string]interface{}{ - // invalid local connector - map[string]interface{}{ - "type": "local", - "passwordInfos": "invalid", - }, - // no type map[string]interface{}{ "id": "bar", diff --git a/connector/connector_local.go b/connector/connector_local.go index 00fd0402..490904d8 100644 --- a/connector/connector_local.go +++ b/connector/connector_local.go @@ -21,8 +21,7 @@ func init() { } type LocalConnectorConfig struct { - ID string `json:"id"` - PasswordInfos []user.PasswordInfo `json:"passwordInfos"` + ID string `json:"id"` } func (cfg *LocalConnectorConfig) ConnectorID() string { diff --git a/integration/oidc_test.go b/integration/oidc_test.go index 8c092953..48e7d862 100644 --- a/integration/oidc_test.go +++ b/integration/oidc_test.go @@ -113,7 +113,7 @@ func TestHTTPExchangeTokenRefreshToken(t *testing.T) { } cfg := &connector.LocalConnectorConfig{ - PasswordInfos: []user.PasswordInfo{passwordInfo}, + ID: "local", } ci := oidc.ClientIdentity{ @@ -128,6 +128,10 @@ func TestHTTPExchangeTokenRefreshToken(t *testing.T) { if err != nil { t.Fatalf("Failed to create client identity repo: " + err.Error()) } + passwordInfoRepo, err := db.NewPasswordInfoRepoFromPasswordInfos(db.NewMemDB(), []user.PasswordInfo{passwordInfo}) + if err != nil { + t.Fatalf("Failed to create password info repo: %v", err) + } issuerURL := url.URL{Scheme: "http", Host: "server.example.com"} sm := manager.NewSessionManager(db.NewSessionRepo(dbMap), db.NewSessionKeyRepo(dbMap)) @@ -153,7 +157,6 @@ func TestHTTPExchangeTokenRefreshToken(t *testing.T) { t.Fatalf("Unexpected error: %v", err) } - passwordInfoRepo := db.NewPasswordInfoRepo(db.NewMemDB()) refreshTokenRepo := refreshtest.NewTestRefreshTokenRepo() srv := &server.Server{ diff --git a/server/config.go b/server/config.go index 243d1a00..6900d120 100644 --- a/server/config.go +++ b/server/config.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "html/template" + "io" "net/url" "os" "path/filepath" @@ -136,7 +137,7 @@ func (cfg *SingleServerConfig) Configure(srv *Server) error { skRepo := db.NewSessionKeyRepo(dbMap) sm := sessionmanager.NewSessionManager(sRepo, skRepo) - users, err := loadUsers(cfg.UsersFile) + users, pwis, err := loadUsers(cfg.UsersFile) if err != nil { return fmt.Errorf("unable to read users from file: %v", err) } @@ -145,7 +146,10 @@ func (cfg *SingleServerConfig) Configure(srv *Server) error { return err } - pwiRepo := db.NewPasswordInfoRepo(dbMap) + pwiRepo, err := db.NewPasswordInfoRepoFromPasswordInfos(dbMap, pwis) + if err != nil { + return err + } refTokRepo := db.NewRefreshTokenRepo(dbMap) @@ -163,28 +167,61 @@ func (cfg *SingleServerConfig) Configure(srv *Server) error { return nil } -func loadUsers(filepath string) (users []user.UserWithRemoteIdentities, err error) { +// loadUsers parses the user.json file and returns the users to be created. +func loadUsers(filepath string) ([]user.UserWithRemoteIdentities, []user.PasswordInfo, error) { f, err := os.Open(filepath) if err != nil { - return + return nil, nil, err } defer f.Close() - err = json.NewDecoder(f).Decode(&users) + return loadUsersFromReader(f) +} + +func loadUsersFromReader(r io.Reader) (users []user.UserWithRemoteIdentities, pwis []user.PasswordInfo, err error) { + // Encoding used by the user config file. + var configUsers []struct { + user.User + Password string `json:"password"` + RemoteIdentities []user.RemoteIdentity `json:"remoteIdentities"` + } + if err := json.NewDecoder(r).Decode(&configUsers); err != nil { + return nil, nil, err + } + + users = make([]user.UserWithRemoteIdentities, len(configUsers)) + pwis = make([]user.PasswordInfo, len(configUsers)) + + for i, u := range configUsers { + users[i] = user.UserWithRemoteIdentities{ + User: u.User, + RemoteIdentities: u.RemoteIdentities, + } + hashedPassword, err := user.NewPasswordFromPlaintext(u.Password) + if err != nil { + return nil, nil, err + } + pwis[i] = user.PasswordInfo{UserID: u.ID, Password: hashedPassword} + } return } +// loadClients parses the clients.json file and returns the clients to be created. func loadClients(filepath string) ([]oidc.ClientIdentity, error) { f, err := os.Open(filepath) if err != nil { return nil, err } defer f.Close() + return loadClientsFromReader(f) +} + +func loadClientsFromReader(r io.Reader) ([]oidc.ClientIdentity, error) { var c []struct { ID string `json:"id"` Secret string `json:"secret"` RedirectURLs []string `json:"redirectURLs"` } - if err := json.NewDecoder(f).Decode(&c); err != nil { + if err := json.NewDecoder(r).Decode(&c); err != nil { return nil, err } clients := make([]oidc.ClientIdentity, len(c)) diff --git a/server/config_test.go b/server/config_test.go new file mode 100644 index 00000000..70ad677d --- /dev/null +++ b/server/config_test.go @@ -0,0 +1,78 @@ +package server + +import ( + "strings" + "testing" + + "github.com/coreos/dex/user" + "github.com/kylelemons/godebug/pretty" +) + +func TestLoadUsers(t *testing.T) { + tests := []struct { + // The raw JSON file + raw string + expUsers []user.UserWithRemoteIdentities + // userid -> plaintext password + expPasswds map[string]string + }{ + { + raw: `[ + { + "id": "elroy-id", + "email": "elroy77@example.com", + "displayName": "Elroy Jonez", + "password": "bones", + "remoteIdentities": [ + { + "connectorId": "local", + "id": "elroy-id" + } + ] + } + ]`, + expUsers: []user.UserWithRemoteIdentities{ + { + User: user.User{ + ID: "elroy-id", + Email: "elroy77@example.com", + DisplayName: "Elroy Jonez", + }, + RemoteIdentities: []user.RemoteIdentity{ + { + ConnectorID: "local", + ID: "elroy-id", + }, + }, + }, + }, + expPasswds: map[string]string{ + "elroy-id": "bones", + }, + }, + } + + for i, tt := range tests { + users, pwInfos, err := loadUsersFromReader(strings.NewReader(tt.raw)) + if err != nil { + t.Errorf("case %d: failed to load user: %v", i, err) + return + } + + if diff := pretty.Compare(tt.expUsers, users); diff != "" { + t.Errorf("case: %d: wantUsers!=gotUsers: %s", i, diff) + } + + // For each password info loaded, verify the password. + for _, pwInfo := range pwInfos { + expPW, ok := tt.expPasswds[pwInfo.UserID] + if !ok { + t.Errorf("no password entry for %s", pwInfo.UserID) + continue + } + if _, err := pwInfo.Authenticate(expPW); err != nil { + t.Errorf("case %d: user %s's password did not match", i, pwInfo.UserID) + } + } + } +} diff --git a/server/server.go b/server/server.go index a8f61f42..f6669c5e 100644 --- a/server/server.go +++ b/server/server.go @@ -178,19 +178,6 @@ func (s *Server) AddConnector(cfg connector.ConnectorConfig) error { UserRepo: s.UserRepo, PasswordInfoRepo: s.PasswordInfoRepo, }) - - localCfg, ok := cfg.(*connector.LocalConnectorConfig) - if !ok { - return errors.New("config for LocalConnector not a LocalConnectorConfig?") - } - - if len(localCfg.PasswordInfos) > 0 { - err := user.LoadPasswordInfos(s.PasswordInfoRepo, - localCfg.PasswordInfos) - if err != nil { - return err - } - } } log.Infof("Loaded IdP connector: id=%s type=%s", connectorID, cfg.ConnectorType()) diff --git a/static/fixtures/connectors.json.sample b/static/fixtures/connectors.json.sample index 681ed3e9..94524465 100644 --- a/static/fixtures/connectors.json.sample +++ b/static/fixtures/connectors.json.sample @@ -1,17 +1,7 @@ [ { "type": "local", - "id": "local", - "passwordInfos": [ - { - "userId":"elroy-id", - "passwordPlaintext": "bones" - }, - { - "userId":"penny", - "passwordPlaintext": "kibble" - } - ] + "id": "local" }, { "type": "oidc", diff --git a/static/fixtures/users.json.sample b/static/fixtures/users.json.sample index 610034a0..0949f12e 100644 --- a/static/fixtures/users.json.sample +++ b/static/fixtures/users.json.sample @@ -1,10 +1,9 @@ [ { - "user":{ - "id": "elroy-id", - "email": "elroy77@example.com", - "displayName": "Elroy Jonez" - }, + "id": "elroy-id", + "email": "elroy77@example.com", + "displayName": "Elroy Jonez", + "password": "bones", "remoteIdentities": [ { "connectorId": "local",