fix #3102: fix race in test

meta/store.go

@@ -99,6 +99,10 @@ type Store struct {
 	// Authentication cache.
 	authCache map[string]string
 
+	// hashPassword generates a cryptographically secure hash for password.
+	// Returns an error if the password is invalid or a hash cannot be generated.
+	hashPassword HashPasswordFn
+
 	Logger *log.Logger
 }
 
@@ -120,7 +124,10 @@ func NewStore(c Config) *Store {
 		LeaderLeaseTimeout: time.Duration(c.LeaderLeaseTimeout),
 		CommitTimeout:      time.Duration(c.CommitTimeout),
 		authCache:          make(map[string]string, 0),
-		Logger:             log.New(os.Stderr, "", log.LstdFlags),
+		hashPassword: func(password string) ([]byte, error) {
+			return bcrypt.GenerateFromPassword([]byte(password), BcryptCost)
+		},
+		Logger: log.New(os.Stderr, "", log.LstdFlags),
 	}
 }
 
@@ -989,6 +996,9 @@ func (s *Store) AdminUserExists() (exists bool, err error) {
 	return
 }
 
+// ErrAuthenticate is returned when authentication fails.
+var ErrAuthenticate = errors.New("authentication failed")
+
 // Authenticate retrieves a user with a matching username and password.
 func (s *Store) Authenticate(username, password string) (ui *UserInfo, err error) {
 	err = s.read(func(data *Data) error {
@@ -999,13 +1009,17 @@ func (s *Store) Authenticate(username, password string) (ui *UserInfo, err error
 		}
 
 		// Check the local auth cache first.
-		if p, ok := s.authCache[username]; ok && p == password {
-			ui = u
-			return nil
+		if p, ok := s.authCache[username]; ok {
+			if p == password {
+				ui = u
+				return nil
+			} else {
+				return ErrAuthenticate
+			}
 		}
 		// Compare password with user hash.
 		if err := bcrypt.CompareHashAndPassword([]byte(u.Hash), []byte(password)); err != nil {
-			return err
+			return ErrAuthenticate
 		}
 
 		s.authCache[username] = password
@@ -1019,7 +1033,7 @@ func (s *Store) Authenticate(username, password string) (ui *UserInfo, err error
 // CreateUser creates a new user in the store.
 func (s *Store) CreateUser(name, password string, admin bool) (*UserInfo, error) {
 	// Hash the password before serializing it.
-	hash, err := HashPassword(password)
+	hash, err := s.hashPassword(password)
 	if err != nil {
 		return nil, err
 	}
@@ -1049,7 +1063,7 @@ func (s *Store) DropUser(name string) error {
 // UpdateUser updates an existing user in the store.
 func (s *Store) UpdateUser(name, password string) error {
 	// Hash the password before serializing it.
-	hash, err := HashPassword(password)
+	hash, err := s.hashPassword(password)
 	if err != nil {
 		return err
 	}
@@ -1312,6 +1326,27 @@ func (s *Store) sync(index uint64, timeout time.Duration) error {
 	}
 }
 
+// BcryptCost is the cost associated with generating password with Bcrypt.
+// This setting is lowered during testing to improve test suite performance.
+var BcryptCost = 10
+
+// HashPasswordFn represnets a password hashing function.
+type HashPasswordFn func(password string) ([]byte, error)
+
+// GetHashPasswordFn returns the current password hashing function.
+func (s *Store) GetHashPasswordFn() HashPasswordFn {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	return s.hashPassword
+}
+
+// SetHashPasswordFn sets the password hashing function.
+func (s *Store) SetHashPasswordFn(fn HashPasswordFn) {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	s.hashPassword = fn
+}
+
 // storeFSM represents the finite state machine used by Store to interact with Raft.
 type storeFSM Store
 
@@ -1750,18 +1785,6 @@ func (rpu *RetentionPolicyUpdate) SetName(v string)            { rpu.Name = &v }
 func (rpu *RetentionPolicyUpdate) SetDuration(v time.Duration) { rpu.Duration = &v }
 func (rpu *RetentionPolicyUpdate) SetReplicaN(v int)           { rpu.ReplicaN = &v }
 
-// BcryptCost is the cost associated with generating password with Bcrypt.
-// This setting is lowered during testing to improve test suite performance.
-var BcryptCost = 10
-
-// HashPassword generates a cryptographically secure hash for password.
-// Returns an error if the password is invalid or a hash cannot be generated.
-var HashPassword = func(password string) ([]byte, error) {
-	// The second arg is the cost of the hashing, higher is slower but makes
-	// it harder to brute force, since it will be really slow and impractical
-	return bcrypt.GenerateFromPassword([]byte(password), BcryptCost)
-}
-
 // assert will panic with a given formatted message if the given condition is false.
 func assert(condition bool, msg string, v ...interface{}) {
 	if !condition {

meta/store_test.go

@@ -20,13 +20,6 @@ import (
 	"golang.org/x/crypto/bcrypt"
 )
 
-func init() {
-	// Disable password hashing to speed up testing.
-	meta.HashPassword = func(password string) ([]byte, error) {
-		return []byte(password), nil
-	}
-}
-
 // Ensure the store returns an error
 func TestStore_Open_ErrStoreOpen(t *testing.T) {
 	t.Parallel()
@@ -668,12 +661,10 @@ func TestStore_Authentication(t *testing.T) {
 	s := MustOpenStore()
 	defer s.Close()
 
-	// Set the hash function back to the real thing for this test.
-	oldHashFn := meta.HashPassword
-	meta.HashPassword = func(password string) ([]byte, error) {
+	// Set the password hash function to the real thing for this test.
+	s.SetHashPasswordFn(func(password string) ([]byte, error) {
 		return bcrypt.GenerateFromPassword([]byte(password), 4)
-	}
-	defer func() { meta.HashPassword = oldHashFn }()
+	})
 
 	// Create user.
 	s.CreateUser("susy", "pass", true)
@@ -826,6 +817,7 @@ func NewStore(c meta.Config) *Store {
 		Store: meta.NewStore(c),
 	}
 	s.Logger = log.New(&s.Stderr, "", log.LstdFlags)
+	s.SetHashPasswordFn(mockHashPassword)
 	return s
 }
 
@@ -990,3 +982,8 @@ func MustTempFile() string {
 	os.Remove(f.Name())
 	return f.Name()
 }
+
+// mockHashPassword is used for most tests to avoid slow calls to bcrypt.
+func mockHashPassword(password string) ([]byte, error) {
+	return []byte(password), nil
+}