Upstream/harbor
1
0
Fork 0
mirror of https://github.com/goharbor/harbor.git synced 2024-07-01 17:25:06 +02:00
Code Issues Projects Releases Wiki Activity

Merge pull request #5030 from ywk253100/180522_change_password

Browse Source 
Update change password API
This commit is contained in:
Daniel Jiang 2018-05-25 11:05:12 +08:00 committed by GitHub
commit 4eec586425
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 197 additions and 223 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

71
src/common/dao/dao_test.go
View File

@ -204,41 +204,6 @@ func TestRegister(t *testing.T) {
}
}
func TestCheckUserPassword(t *testing.T) {
nonExistUser := models.User{
Username: "non-exist",
}
correctUser := models.User{
Username: username,
Password: password,
}
wrongPwd := models.User{
Username: username,
Password: "wrong",
}
u, err := CheckUserPassword(nonExistUser)
if err != nil {
t.Errorf("Failed in CheckUserPassword: %v", err)
}
if u != nil {
t.Errorf("Expected nil for Non exist user, but actual: %+v", u)
}
u, err = CheckUserPassword(wrongPwd)
if err != nil {
t.Errorf("Failed in CheckUserPassword: %v", err)
}
if u != nil {
t.Errorf("Expected nil for user with wrong password, but actual: %+v", u)
}
u, err = CheckUserPassword(correctUser)
if err != nil {
t.Errorf("Failed in CheckUserPassword: %v", err)
}
if u == nil {
t.Errorf("User should not be nil for correct user")
}
}
func TestUserExists(t *testing.T) {
var exists bool
var err error
@ -397,42 +362,6 @@ func TestChangeUserPassword(t *testing.T) {
t.Errorf("The username returned by Login does not match, expected: %s, acutal: %s", username, loginedUser.Username)
}
}
func TestChangeUserPasswordWithOldPassword(t *testing.T) {
user := models.User{UserID: currentUser.UserID}
query, err := GetUser(user)
if err != nil {
t.Errorf("Error occurred when get user salt")
}
currentUser.Salt = query.Salt
err = ChangeUserPassword(models.User{UserID: currentUser.UserID, Password: "NewerHarborTester12345", Salt: currentUser.Salt}, "NewHarborTester12345")
if err != nil {
t.Errorf("Error occurred in ChangeUserPassword: %v", err)
}
loginedUser, err := LoginByDb(models.AuthModel{Principal: currentUser.Username, Password: "NewerHarborTester12345"})
if err != nil {
t.Errorf("Error occurred in LoginByDb: %v", err)
}
if loginedUser.Username != username {
t.Errorf("The username returned by Login does not match, expected: %s, acutal: %s", username, loginedUser.Username)
}
}
func TestChangeUserPasswordWithIncorrectOldPassword(t *testing.T) {
err := ChangeUserPassword(models.User{UserID: currentUser.UserID, Password: "NNewerHarborTester12345", Salt: currentUser.Salt}, "WrongNewerHarborTester12345")
if err == nil {
t.Errorf("Error does not occurred due to old password is incorrect.")
}
loginedUser, err := LoginByDb(models.AuthModel{Principal: currentUser.Username, Password: "NNewerHarborTester12345"})
if err != nil {
t.Errorf("Error occurred in LoginByDb: %v", err)
}
if loginedUser != nil {
t.Errorf("The login user is not nil, acutal: %+v", loginedUser)
}
}
func TestAddProject(t *testing.T) {
project := models.Project{

68
src/common/dao/user.go
View File

@ -15,9 +15,9 @@
package dao
import (
"database/sql"
"errors"
"fmt"
"time"
"github.com/astaxie/beego/orm"
@ -32,7 +32,7 @@ func GetUser(query models.User) (*models.User, error) {
o := GetOrmer()
sql := `select user_id, username, email, realname, comment, reset_uuid, salt,
sql := `select user_id, username, password, email, realname, comment, reset_uuid, salt,
sysadmin_flag, creation_time, update_time
from harbor_user u
where deleted = false `
@ -153,34 +153,12 @@ func ToggleUserAdminRole(userID int, hasAdmin bool) error {
}
// ChangeUserPassword ...
func ChangeUserPassword(u models.User, oldPassword ...string) (err error) {
if len(oldPassword) > 1 {
return errors.New("wrong numbers of params")
}
o := GetOrmer()
var r sql.Result
salt := utils.GenerateRandomString()
if len(oldPassword) == 0 {
//In some cases, it may no need to check old password, just as Linux change password policies.
r, err = o.Raw(`update harbor_user set password=?, salt=? where user_id=?`, utils.Encrypt(u.Password, salt), salt, u.UserID).Exec()
} else {
r, err = o.Raw(`update harbor_user set password=?, salt=? where user_id=? and password = ?`, utils.Encrypt(u.Password, salt), salt, u.UserID, utils.Encrypt(oldPassword[0], u.Salt)).Exec()
}
if err != nil {
return err
}
c, err := r.RowsAffected()
if err != nil {
return err
}
if c == 0 {
return errors.New("no record has been modified, change password failed")
}
return nil
func ChangeUserPassword(u models.User) error {
u.UpdateTime = time.Now()
u.Salt = utils.GenerateRandomString()
u.Password = utils.Encrypt(u.Password, u.Salt)
_, err := GetOrmer().Update(&u, "Password", "Salt", "UpdateTime")
return err
}
// ResetUserPassword ...
@ -207,36 +185,6 @@ func UpdateUserResetUUID(u models.User) error {
return err
}
// CheckUserPassword checks whether the password is correct.
func CheckUserPassword(query models.User) (*models.User, error) {
currentUser, err := GetUser(query)
if err != nil {
return nil, err
}
if currentUser == nil {
return nil, nil
}
sql := `select user_id, username, salt from harbor_user where deleted = false and username = ? and password = ?`
queryParam := make([]interface{}, 1)
queryParam = append(queryParam, currentUser.Username)
queryParam = append(queryParam, utils.Encrypt(query.Password, currentUser.Salt))
o := GetOrmer()
var user []models.User
n, err := o.Raw(sql, queryParam).QueryRows(&user)
if err != nil {
return nil, err
}
if n == 0 {
log.Warning("User principal does not match password. Current:", currentUser)
return nil, nil
}
return &user[0], nil
}
// DeleteUser ...
func DeleteUser(userID int) error {
o := GetOrmer()

59
src/ui/api/user.go
View File

@ -24,6 +24,7 @@ import (
"github.com/vmware/harbor/src/common"
"github.com/vmware/harbor/src/common/dao"
"github.com/vmware/harbor/src/common/models"
"github.com/vmware/harbor/src/common/utils"
"github.com/vmware/harbor/src/common/utils/log"
"github.com/vmware/harbor/src/ui/config"
)
@ -114,6 +115,7 @@ func (ua *UserAPI) Get() {
log.Errorf("Error occurred in GetUser, error: %v", err)
ua.CustomAbort(http.StatusInternalServerError, "Internal error.")
}
u.Password = ""
ua.Data["json"] = u
ua.ServeJSON()
return
@ -273,33 +275,48 @@ func (ua *UserAPI) ChangePassword() {
return
}
changePwdOfOwn := ua.userID == ua.currentUserID
var req passwordReq
ua.DecodeJSONReq(&req)
if req.OldPassword == "" {
log.Error("Old password is blank")
ua.CustomAbort(http.StatusBadRequest, "Old password is blank")
}
queryUser := models.User{UserID: ua.userID, Password: req.OldPassword}
user, err := dao.CheckUserPassword(queryUser)
if err != nil {
log.Errorf("Error occurred in CheckUserPassword: %v", err)
ua.CustomAbort(http.StatusInternalServerError, "Internal error.")
}
if user == nil {
log.Warning("Password input is not correct")
ua.CustomAbort(http.StatusForbidden, "old_password_is_not_correct")
}
if req.NewPassword == "" {
ua.HandleBadRequest("new password is null")
if changePwdOfOwn && len(req.OldPassword) == 0 {
ua.HandleBadRequest("empty old_password")
return
}
updateUser := models.User{UserID: ua.userID, Password: req.NewPassword, Salt: user.Salt}
err = dao.ChangeUserPassword(updateUser, req.OldPassword)
if len(req.NewPassword) == 0 {
ua.HandleBadRequest("empty new_password")
return
}
user, err := dao.GetUser(models.User{UserID: ua.userID})
if err != nil {
log.Errorf("Error occurred in ChangeUserPassword: %v", err)
ua.CustomAbort(http.StatusInternalServerError, "Internal error.")
ua.HandleInternalServerError(fmt.Sprintf("failed to get user %d: %v", ua.userID, err))
return
}
if user == nil {
ua.HandleNotFound(fmt.Sprintf("user %d not found", ua.userID))
return
}
if changePwdOfOwn {
if user.Password != utils.Encrypt(req.OldPassword, user.Salt) {
ua.HandleForbidden("incorrect old_password")
return
}
}
if user.Password == utils.Encrypt(req.NewPassword, user.Salt) {
ua.HandleBadRequest("the new password can not be same with the old one")
return
}
updatedUser := models.User{
UserID: ua.userID,
Password: req.NewPassword,
}
if err = dao.ChangeUserPassword(updatedUser); err != nil {
ua.HandleInternalServerError(fmt.Sprintf("failed to change password of user %d: %v", ua.userID, err))
return
}
}

222
src/ui/api/user_test.go
View File

@ -15,10 +15,15 @@ package api
import (
"fmt"
"net/http"
"testing"
"github.com/stretchr/testify/require"
"github.com/vmware/harbor/src/common/dao"
"github.com/stretchr/testify/assert"
"github.com/vmware/harbor/src/common/api"
"github.com/vmware/harbor/src/common/models"
"github.com/vmware/harbor/tests/apitests/apilib"
"github.com/astaxie/beego"
@ -323,82 +328,157 @@ func TestUsersToggleAdminRole(t *testing.T) {
assert.Equal(200, code, "Toggle user admin role status should be 200")
}
}
func buildChangeUserPasswordURL(id int) string {
return fmt.Sprintf("/api/users/%d/password", id)
}
func TestUsersUpdatePassword(t *testing.T) {
fmt.Println("Testing Update User Password")
assert := assert.New(t)
apiTest := newHarborAPI()
password := apilib.Password{OldPassword: "", NewPassword: ""}
t.Log("Update password-case 1")
//case 1: update user2 password with user3 auth
code, err := apiTest.UsersUpdatePassword(testUser0002ID, password, *testUser0003Auth)
if err != nil {
t.Error("Error occured while update user password", err.Error())
t.Log(err)
} else {
assert.Equal(403, code, "Update user password status should be 403")
}
t.Log("Update password-case 2")
//case 2: update user2 password with admin auth, but oldpassword is empty
code, err = apiTest.UsersUpdatePassword(testUser0002ID, password, *admin)
if err != nil {
t.Error("Error occured while update user password", err.Error())
t.Log(err)
} else {
assert.Equal(400, code, "Update user password status should be 400")
}
t.Log("Update password-case 3")
//case 3: update user2 password with admin auth, but oldpassword is wrong
password.OldPassword = "000"
code, err = apiTest.UsersUpdatePassword(testUser0002ID, password, *admin)
if err != nil {
t.Error("Error occured while update user password", err.Error())
t.Log(err)
} else {
assert.Equal(403, code, "Update user password status should be 403")
}
t.Log("Update password-case 4")
//case 4: update user2 password with admin auth, but newpassword is empty
password.OldPassword = "testUser0002"
code, err = apiTest.UsersUpdatePassword(testUser0002ID, password, *admin)
if err != nil {
t.Error("Error occured while update user password", err.Error())
t.Log(err)
} else {
assert.Equal(400, code, "Update user password status should be 400")
}
t.Log("Update password-case 5")
//case 5: update user2 password with admin auth, right parameters
password.NewPassword = "TestUser0002"
code, err = apiTest.UsersUpdatePassword(testUser0002ID, password, *admin)
if err != nil {
t.Error("Error occured while update user password", err.Error())
t.Log(err)
} else {
assert.Equal(200, code, "Update user password status should be 200")
testUser0002.Password = password.NewPassword
testUser0002Auth.Passwd = password.NewPassword
//verify the new password takes effect
code, user, err := apiTest.UsersGetByID(testUser0002.Username, *testUser0002Auth, testUser0002ID)
if err != nil {
t.Error("Error occured while get users", err.Error())
t.Log(err)
} else {
assert.Equal(200, code, "Get users status should be 200")
assert.Equal(testUser0002.Username, user.Username, "Get users username should be equal")
assert.Equal(testUser0002.Email, user.Email, "Get users email should be equal")
}
oldPassword := "old_password"
newPassword := "new_password"
user01 := models.User{
Username: "user01_for_testing_change_password",
Email: "user01_for_testing_change_password@test.com",
Password: oldPassword,
}
t.Log("Update password-case 6")
//case 6: update user2 password setting the new password same as the old
password.OldPassword = password.NewPassword
code, err = apiTest.UsersUpdatePassword(testUser0002ID, password, *admin)
if err != nil {
t.Error("Error occured while update user password", err.Error())
t.Log(err)
} else {
assert.Equal(200, code, "When new password is same as old, update user password status should be 200")
id, err := dao.Register(user01)
require.Nil(t, err)
user01.UserID = int(id)
defer dao.DeleteUser(user01.UserID)
user02 := models.User{
Username: "user02_for_testing_change_password",
Email: "user02_for_testing_change_password@test.com",
Password: oldPassword,
}
id, err = dao.Register(user02)
require.Nil(t, err)
user02.UserID = int(id)
defer dao.DeleteUser(user02.UserID)
cases := []*codeCheckingCase{
// unauthorized
&codeCheckingCase{
request: &testingRequest{
method: http.MethodPut,
url: buildChangeUserPasswordURL(user01.UserID),
},
code: http.StatusUnauthorized,
},
// 404
&codeCheckingCase{
request: &testingRequest{
method: http.MethodPut,
url: buildChangeUserPasswordURL(10000),
credential: &usrInfo{
Name: user01.Username,
Passwd: user01.Password,
},
},
code: http.StatusNotFound,
},
// 403, a normal user tries to change password of others
&codeCheckingCase{
request: &testingRequest{
method: http.MethodPut,
url: buildChangeUserPasswordURL(user02.UserID),
credential: &usrInfo{
Name: user01.Username,
Passwd: user01.Password,
},
},
code: http.StatusForbidden,
},
// 400, empty old password
&codeCheckingCase{
request: &testingRequest{
method: http.MethodPut,
url: buildChangeUserPasswordURL(user01.UserID),
bodyJSON: &passwordReq{},
credential: &usrInfo{
Name: user01.Username,
Passwd: user01.Password,
},
},
code: http.StatusBadRequest,
},
// 400, empty new password
&codeCheckingCase{
request: &testingRequest{
method: http.MethodPut,
url: buildChangeUserPasswordURL(user01.UserID),
bodyJSON: &passwordReq{
OldPassword: oldPassword,
},
credential: &usrInfo{
Name: user01.Username,
Passwd: user01.Password,
},
},
code: http.StatusBadRequest,
},
// 403, incorrect old password
&codeCheckingCase{
request: &testingRequest{
method: http.MethodPut,
url: buildChangeUserPasswordURL(user01.UserID),
bodyJSON: &passwordReq{
OldPassword: "incorrect_old_password",
NewPassword: newPassword,
},
credential: &usrInfo{
Name: user01.Username,
Passwd: user01.Password,
},
},
code: http.StatusForbidden,
},
// 200, normal user change own password
&codeCheckingCase{
request: &testingRequest{
method: http.MethodPut,
url: buildChangeUserPasswordURL(user01.UserID),
bodyJSON: &passwordReq{
OldPassword: oldPassword,
NewPassword: newPassword,
},
credential: &usrInfo{
Name: user01.Username,
Passwd: user01.Password,
},
},
code: http.StatusOK,
},
// 400, admin user change password of others.
// the new password is same with the old one
&codeCheckingCase{
request: &testingRequest{
method: http.MethodPut,
url: buildChangeUserPasswordURL(user01.UserID),
bodyJSON: &passwordReq{
NewPassword: newPassword,
},
credential: admin,
},
code: http.StatusBadRequest,
},
// 200, admin user change password of others
&codeCheckingCase{
request: &testingRequest{
method: http.MethodPut,
url: buildChangeUserPasswordURL(user01.UserID),
bodyJSON: &passwordReq{
NewPassword: "another_new_password",
},
credential: admin,
},
code: http.StatusOK,
},
}
runCodeCheckingCases(t, cases...)
}
func TestUsersDelete(t *testing.T) {