diff --git a/controllers/password.go b/controllers/password.go index e6211d413..9ecc6d713 100644 --- a/controllers/password.go +++ b/controllers/password.go @@ -43,15 +43,19 @@ func (cpc *ChangePasswordController) Get() { func (cpc *CommonController) UpdatePassword() { sessionUserId := cpc.GetSession("userId") - sessionUsername := cpc.GetSession("username") - if sessionUserId == nil || sessionUsername == nil { + if sessionUserId == nil { beego.Warning("User does not login.") cpc.CustomAbort(401, "please_login_first") } oldPassword := cpc.GetString("old_password") - queryUser := models.User{UserId: sessionUserId.(int), Username: sessionUsername.(string), Password: oldPassword} + if oldPassword == "" { + beego.Error("Old password is blank") + cpc.CustomAbort(400, "Old password is blank") + } + + queryUser := models.User{UserId: sessionUserId.(int), Password: oldPassword} user, err := dao.CheckUserPassword(queryUser) if err != nil { beego.Error("Error occurred in CheckUserPassword:", err) @@ -65,10 +69,14 @@ func (cpc *CommonController) UpdatePassword() { password := cpc.GetString("password") if password != "" { - updateUser := models.User{UserId: sessionUserId.(int), Username: sessionUsername.(string), Password: password, Salt: user.Salt} - dao.ChangeUserPassword(updateUser) + updateUser := models.User{UserId: sessionUserId.(int), Password: password, Salt: user.Salt} + err = dao.ChangeUserPassword(updateUser, oldPassword) + if err != nil { + beego.Error("Error occurred in ChangeUserPassword:", err) + cpc.CustomAbort(500, "Internal error.") + } } else { - cpc.CustomAbort(404, "please_input_new_password") + cpc.CustomAbort(400, "please_input_new_password") } } @@ -90,7 +98,11 @@ func (fpc *CommonController) SendEmail() { email := fpc.GetString("email") - if ok, _ := regexp.MatchString(`^(([^<>()[\]\\.,;:\s@\"]+(\.[^<>()[\]\\.,;:\s@\"]+)*)|(\".+\"))@((\[[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\])|(([a-zA-Z\-0-9]+\.)+[a-zA-Z]{2,}))$`, email); ok { + pass, _ := regexp.MatchString(`^(([^<>()[\]\\.,;:\s@\"]+(\.[^<>()[\]\\.,;:\s@\"]+)*)|(\".+\"))@((\[[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\])|(([a-zA-Z\-0-9]+\.)+[a-zA-Z]{2,}))$`, email) + + if !pass { + fpc.CustomAbort(400, "email_content_illegal") + } else { queryUser := models.User{Email: email} exist, err := dao.UserExists(queryUser, "email") @@ -152,8 +164,6 @@ func (fpc *CommonController) SendEmail() { user := models.User{ResetUuid: uuid, Email: email} dao.UpdateUserResetUuid(user) - } else { - fpc.CustomAbort(409, "email_content_illegal") } } @@ -164,8 +174,13 @@ type ResetPasswordController struct { func (rpc *ResetPasswordController) Get() { - q := rpc.GetString("q") - queryUser := models.User{ResetUuid: q} + resetUuid := rpc.GetString("reset_uuid") + if resetUuid == "" { + beego.Error("Reset uuid is blank.") + rpc.Redirect("/", 302) + } + + queryUser := models.User{ResetUuid: resetUuid} user, err := dao.GetUser(queryUser) if err != nil { beego.Error("Error occurred in GetUser:", err) @@ -183,6 +198,9 @@ func (rpc *ResetPasswordController) Get() { func (rpc *CommonController) ResetPassword() { resetUuid := rpc.GetString("reset_uuid") + if resetUuid == "" { + rpc.CustomAbort(400, "Reset uuid is blank.") + } queryUser := models.User{ResetUuid: resetUuid} user, err := dao.GetUser(queryUser) @@ -190,13 +208,21 @@ func (rpc *CommonController) ResetPassword() { beego.Error("Error occurred in GetUser:", err) rpc.CustomAbort(500, "Internal error.") } + if user == nil { + beego.Error("User does not exist") + rpc.CustomAbort(400, "User does not exist") + } password := rpc.GetString("password") if password != "" { user.Password = password - dao.ResetUserPassword(*user) + err = dao.ResetUserPassword(*user) + if err != nil { + beego.Error("Error occurred in ResetUserPassword:", err) + rpc.CustomAbort(500, "Internal error.") + } } else { - rpc.CustomAbort(404, "password_is_required") + rpc.CustomAbort(400, "password_is_required") } } diff --git a/dao/user.go b/dao/user.go index e215712f4..a0a259cc5 100644 --- a/dao/user.go +++ b/dao/user.go @@ -15,6 +15,9 @@ package dao import ( + "database/sql" + "errors" + "github.com/vmware/harbor/models" "github.com/vmware/harbor/utils" @@ -133,15 +136,44 @@ func ToggleUserAdminRole(u models.User) error { return err } -func ChangeUserPassword(u models.User) error { +func ChangeUserPassword(u models.User, oldPassword ...string) error { o := orm.NewOrm() - _, err := o.Raw(`update user set password=?, salt=? where user_id=?`, utils.Encrypt(u.Password, u.Salt), u.Salt, u.UserId).Exec() + var err error + var r sql.Result + if len(oldPassword) == 0 { + //In some cases, it may no need to check old password, just as Linux change password policies. + _, err = o.Raw(`update user set password=?, salt=? where user_id=?`, utils.Encrypt(u.Password, u.Salt), u.Salt, u.UserId).Exec() + } else if len(oldPassword) == 1 { + r, err = o.Raw(`update user set password=?, salt=? where user_id=? and password = ?`, utils.Encrypt(u.Password, u.Salt), u.Salt, u.UserId, utils.Encrypt(oldPassword[0], u.Salt)).Exec() + if err != nil { + return err + } + count, err := r.RowsAffected() + if err != nil { + return err + } + if count == 0 { + return errors.New("No record be changed, change password failed.") + } + } else { + return errors.New("Wrong numbers of params.") + } return err } func ResetUserPassword(u models.User) error { o := orm.NewOrm() - _, err := o.Raw(`update user set password=?, reset_uuid=? where reset_uuid=?`, utils.Encrypt(u.Password, u.Salt), "", u.ResetUuid).Exec() + r, err := o.Raw(`update user set password=?, reset_uuid=? where reset_uuid=?`, utils.Encrypt(u.Password, u.Salt), "", u.ResetUuid).Exec() + if err != nil { + return err + } + count, err := r.RowsAffected() + if err != nil { + return err + } + if count == 0 { + return errors.New("No record be changed, reset password failed.") + } return err } diff --git a/main.go b/main.go index c6a69c8c2..531372297 100644 --- a/main.go +++ b/main.go @@ -38,7 +38,7 @@ func updateInitPassword(userId int, password string) error { queryUser := models.User{UserId: userId} user, err := dao.GetUser(queryUser) if err != nil { - log.Println("Failed to get user in initial password, userId:", userId) + log.Println("Failed to get user, userId:", userId) return err } if user == nil { diff --git a/tests/dao_test.go b/tests/dao_test.go index 9a40b7753..0179da876 100644 --- a/tests/dao_test.go +++ b/tests/dao_test.go @@ -15,6 +15,7 @@ package test import ( + "fmt" // "fmt" "log" "os" @@ -112,6 +113,8 @@ func TestMain(m *testing.M) { log.Fatalf("environment variable DB_PWD is not set") } + fmt.Printf("DB_HOST: %s, DB_USR: %s, DB_PORT: %s, DB_PWD: %s\n", dbHost, dbUser, dbPort, dbPassword) + os.Setenv("MYSQL_PORT_3306_TCP_ADDR", dbHost) os.Setenv("MYSQL_PORT_3306_TCP_PORT", dbPort) os.Setenv("MYSQL_USR", dbUser) @@ -300,6 +303,34 @@ func TestChangeUserPassword(t *testing.T) { } } +func TestChangeUserPasswordWithOldPassword(t *testing.T) { + err := dao.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 := dao.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 := dao.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 := dao.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 TestQueryRelevantProjectsWhenNoProjectAdded(t *testing.T) { projects, err := dao.QueryRelevantProjects(currentUser.UserId) if err != nil { diff --git a/views/reset-password-mail.tpl b/views/reset-password-mail.tpl index 35b9c6976..e79cdfddb 100644 --- a/views/reset-password-mail.tpl +++ b/views/reset-password-mail.tpl @@ -16,6 +16,6 @@

{{.Hint}}:

- {{.Url}}/resetPassword?q={{.Uuid}} + {{.Url}}/resetPassword?reset_uuid={{.Uuid}} \ No newline at end of file