From e5b8b4b5ec076b100b86845d2b4a8ff7ff71a87a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Kim=20=22BKC=22=20Carlb=C3=A4cker?=
<kim.carlbacker@gmail.com>
Date: Thu, 11 Jan 2018 23:19:38 +0100
Subject: [PATCH] Cleanup models.User.HashPassword (#3334)
---
cmd/admin.go | 3 +--
models/user.go | 17 ++++++++++-------
models/user_test.go | 9 +++------
routers/admin/users.go | 3 +--
routers/api/v1/admin/user.go | 3 +--
routers/user/auth.go | 3 +--
routers/user/setting.go | 3 +--
7 files changed, 18 insertions(+), 23 deletions(-)
diff --git a/cmd/admin.go b/cmd/admin.go
index 5000fc2d4..33d9daa44 100644
--- a/cmd/admin.go
+++ b/cmd/admin.go
@@ -107,11 +107,10 @@ func runChangePassword(c *cli.Context) error {
if err != nil {
return fmt.Errorf("%v", err)
}
- user.Passwd = c.String("password")
if user.Salt, err = models.GetUserSalt(); err != nil {
return fmt.Errorf("%v", err)
}
- user.HashPassword()
+ user.HashPassword(c.String("password"))
if err := models.UpdateUserCols(user, "passwd", "salt"); err != nil {
return fmt.Errorf("%v", err)
}
diff --git a/models/user.go b/models/user.go
index c30c66d6c..aaf2320b4 100644
--- a/models/user.go
+++ b/models/user.go
@@ -388,17 +388,20 @@ func (u *User) NewGitSig() *git.Signature {
}
}
+func hashPassword(passwd, salt string) string {
+ tempPasswd := pbkdf2.Key([]byte(passwd), []byte(salt), 10000, 50, sha256.New)
+ return fmt.Sprintf("%x", tempPasswd)
+}
+
// HashPassword hashes a password using PBKDF.
-func (u *User) HashPassword() {
- newPasswd := pbkdf2.Key([]byte(u.Passwd), []byte(u.Salt), 10000, 50, sha256.New)
- u.Passwd = fmt.Sprintf("%x", newPasswd)
+func (u *User) HashPassword(passwd string) {
+ u.Passwd = hashPassword(passwd, u.Salt)
}
// ValidatePassword checks if given password matches the one belongs to the user.
func (u *User) ValidatePassword(passwd string) bool {
- newUser := &User{Passwd: passwd, Salt: u.Salt}
- newUser.HashPassword()
- return subtle.ConstantTimeCompare([]byte(u.Passwd), []byte(newUser.Passwd)) == 1
+ tempHash := hashPassword(passwd, u.Salt)
+ return subtle.ConstantTimeCompare([]byte(u.Passwd), []byte(tempHash)) == 1
}
// IsPasswordSet checks if the password is set or left empty
@@ -711,7 +714,7 @@ func CreateUser(u *User) (err error) {
if u.Salt, err = GetUserSalt(); err != nil {
return err
}
- u.HashPassword()
+ u.HashPassword(u.Passwd)
u.AllowCreateOrganization = setting.Service.DefaultAllowCreateOrganization
u.MaxRepoCreation = -1
diff --git a/models/user_test.go b/models/user_test.go
index c0b7dace7..4fd0bc0fa 100644
--- a/models/user_test.go
+++ b/models/user_test.go
@@ -135,13 +135,11 @@ func TestHashPasswordDeterministic(t *testing.T) {
pass := string(b)
// save the current password in the user - hash it and store the result
- u.Passwd = pass
- u.HashPassword()
+ u.HashPassword(pass)
r1 := u.Passwd
// run again
- u.Passwd = pass
- u.HashPassword()
+ u.HashPassword(pass)
r2 := u.Passwd
// assert equal (given the same salt+pass, the same result is produced)
@@ -158,7 +156,6 @@ func BenchmarkHashPassword(b *testing.B) {
u := &User{Salt: string(bs), Passwd: pass}
b.ResetTimer()
for i := 0; i < b.N; i++ {
- u.HashPassword()
- u.Passwd = pass
+ u.HashPassword(pass)
}
}
diff --git a/routers/admin/users.go b/routers/admin/users.go
index eee0c21f4..9aa78db10 100644
--- a/routers/admin/users.go
+++ b/routers/admin/users.go
@@ -194,13 +194,12 @@ func EditUserPost(ctx *context.Context, form auth.AdminEditUserForm) {
}
if len(form.Password) > 0 {
- u.Passwd = form.Password
var err error
if u.Salt, err = models.GetUserSalt(); err != nil {
ctx.ServerError("UpdateUser", err)
return
}
- u.HashPassword()
+ u.HashPassword(form.Password)
}
u.LoginName = form.LoginName
diff --git a/routers/api/v1/admin/user.go b/routers/api/v1/admin/user.go
index 5bbb8a7af..69e1761b0 100644
--- a/routers/api/v1/admin/user.go
+++ b/routers/api/v1/admin/user.go
@@ -126,13 +126,12 @@ func EditUser(ctx *context.APIContext, form api.EditUserOption) {
}
if len(form.Password) > 0 {
- u.Passwd = form.Password
var err error
if u.Salt, err = models.GetUserSalt(); err != nil {
ctx.Error(500, "UpdateUser", err)
return
}
- u.HashPassword()
+ u.HashPassword(form.Password)
}
u.LoginName = form.LoginName
diff --git a/routers/user/auth.go b/routers/user/auth.go
index c3fb911b0..6edcb914b 100644
--- a/routers/user/auth.go
+++ b/routers/user/auth.go
@@ -984,7 +984,6 @@ func ResetPasswdPost(ctx *context.Context) {
return
}
- u.Passwd = passwd
var err error
if u.Rands, err = models.GetUserSalt(); err != nil {
ctx.ServerError("UpdateUser", err)
@@ -994,7 +993,7 @@ func ResetPasswdPost(ctx *context.Context) {
ctx.ServerError("UpdateUser", err)
return
}
- u.HashPassword()
+ u.HashPassword(passwd)
if err := models.UpdateUserCols(u, "passwd", "rands", "salt"); err != nil {
ctx.ServerError("UpdateUser", err)
return
diff --git a/routers/user/setting.go b/routers/user/setting.go
index dcd0eedb8..b674c24b4 100644
--- a/routers/user/setting.go
+++ b/routers/user/setting.go
@@ -229,13 +229,12 @@ func SettingsSecurityPost(ctx *context.Context, form auth.ChangePasswordForm) {
} else if form.Password != form.Retype {
ctx.Flash.Error(ctx.Tr("form.password_not_match"))
} else {
- ctx.User.Passwd = form.Password
var err error
if ctx.User.Salt, err = models.GetUserSalt(); err != nil {
ctx.ServerError("UpdateUser", err)
return
}
- ctx.User.HashPassword()
+ ctx.User.HashPassword(form.Password)
if err := models.UpdateUserCols(ctx.User, "salt", "passwd"); err != nil {
ctx.ServerError("UpdateUser", err)
return