You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@servicecomb.apache.org by ti...@apache.org on 2020/07/13 13:43:52 UTC

[servicecomb-service-center] branch master updated: change password api refine (#665)

This is an automated email from the ASF dual-hosted git repository.

tianxiaoliang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/servicecomb-service-center.git


The following commit(s) were added to refs/heads/master by this push:
     new dc50fed  change password api refine (#665)
dc50fed is described below

commit dc50fed75eb0d96959304f067f9be84e8581a11c
Author: Shawn <xi...@gmail.com>
AuthorDate: Mon Jul 13 21:43:41 2020 +0800

    change password api refine (#665)
---
 server/rest/controller/v4/auth_resource.go      |  3 ++-
 server/rest/controller/v4/auth_resource_test.go | 15 ++++++++++++++-
 server/service/rbac/password.go                 | 20 ++++++++++----------
 server/service/rbac/rbca_test.go                |  2 +-
 server/service/validate.go                      |  2 +-
 5 files changed, 28 insertions(+), 14 deletions(-)

diff --git a/server/rest/controller/v4/auth_resource.go b/server/rest/controller/v4/auth_resource.go
index 6c9d446..dd0cfeb 100644
--- a/server/rest/controller/v4/auth_resource.go
+++ b/server/rest/controller/v4/auth_resource.go
@@ -42,7 +42,7 @@ func (r *AuthResource) URLPatterns() []rest.Route {
 	return []rest.Route{
 		{Method: http.MethodPost, Path: "/v4/token", Func: r.Login},
 		{Method: http.MethodPost, Path: "/v4/account", Func: r.CreateAccount},
-		{Method: http.MethodPut, Path: "/v4/reset-password", Func: r.ChangePassword},
+		{Method: http.MethodPost, Path: "/v4/account/:name/password", Func: r.ChangePassword},
 	}
 }
 func (r *AuthResource) CreateAccount(w http.ResponseWriter, req *http.Request) {
@@ -87,6 +87,7 @@ func (r *AuthResource) ChangePassword(w http.ResponseWriter, req *http.Request)
 		controller.WriteError(w, scerror.ErrInvalidParams, errorsEx.ErrMsgJSON)
 		return
 	}
+	a.Name = req.URL.Query().Get(":name")
 	err = service.ValidateChangePWD(a)
 	if err != nil {
 		controller.WriteError(w, scerror.ErrInvalidParams, err.Error())
diff --git a/server/rest/controller/v4/auth_resource_test.go b/server/rest/controller/v4/auth_resource_test.go
index 9e414ee..4a693d4 100644
--- a/server/rest/controller/v4/auth_resource_test.go
+++ b/server/rest/controller/v4/auth_resource_test.go
@@ -72,12 +72,25 @@ func TestAuthResource_Login(t *testing.T) {
 		rest.GetRouter().ServeHTTP(w, r)
 		assert.Equal(t, http.StatusOK, w.Code)
 	})
-	t.Run("dev_account login", func(t *testing.T) {
+	t.Run("invalid password", func(t *testing.T) {
+		b, _ := json.Marshal(&rbacframe.Account{Name: "root", Password: "Complicated_password"})
+
+		r, _ := http.NewRequest(http.MethodPost, "/v4/token", bytes.NewBuffer(b))
+		w := httptest.NewRecorder()
+		rest.GetRouter().ServeHTTP(w, r)
+		assert.Equal(t, http.StatusUnauthorized, w.Code)
+	})
+	t.Run("dev_account login and change pwd", func(t *testing.T) {
 		b, _ := json.Marshal(&rbacframe.Account{Name: "dev_account", Password: "Complicated_password1"})
 
 		r, _ := http.NewRequest(http.MethodPost, "/v4/token", bytes.NewBuffer(b))
 		w := httptest.NewRecorder()
 		rest.GetRouter().ServeHTTP(w, r)
 		assert.Equal(t, http.StatusOK, w.Code)
+		jsonbody := w.Body.Bytes()
+		to := &rbacframe.Token{}
+		json.Unmarshal(jsonbody, to)
+
 	})
+
 }
diff --git a/server/service/rbac/password.go b/server/service/rbac/password.go
index 1fcbd2b..971f6c2 100644
--- a/server/service/rbac/password.go
+++ b/server/service/rbac/password.go
@@ -28,18 +28,18 @@ import (
 )
 
 func ChangePassword(ctx context.Context, changerRole, changerName string, a *rbacframe.Account) error {
-	if a.Name != "" {
-		if changerRole != rbacframe.RoleAdmin { //need to check password mismatch. but admin role can change any user password without supply current password
-			log.Error("can not change other account pwd", nil)
-			return ErrNoPermChangeAccount
+	if changerName == a.Name {
+		if a.CurrentPassword == "" {
+			log.Error("current pwd is empty", nil)
+			return ErrEmptyCurrentPassword
 		}
-		return changePasswordForcibly(ctx, a.Name, a.Password)
+		return changePassword(ctx, changerName, a.CurrentPassword, a.Password)
 	}
-	if a.CurrentPassword == "" {
-		log.Error("current pwd is empty", nil)
-		return ErrEmptyCurrentPassword
+	if changerRole != rbacframe.RoleAdmin { //need to check password mismatch. but admin role can change any user password without supply current password
+		log.Error("can not change other account pwd", nil)
+		return ErrNoPermChangeAccount
 	}
-	return changePassword(ctx, changerName, a.CurrentPassword, a.Password)
+	return changePasswordForcibly(ctx, a.Name, a.Password)
 
 }
 func changePasswordForcibly(ctx context.Context, name, pwd string) error {
@@ -65,7 +65,7 @@ func changePassword(ctx context.Context, name, currentPassword, pwd string) erro
 	}
 	same := SamePassword(old.Password, currentPassword)
 	if !same {
-		log.Error("current pwd is wrong", nil)
+		log.Error("current password is wrong", nil)
 		return ErrWrongPassword
 	}
 	err = doChangePassword(ctx, old, pwd)
diff --git a/server/service/rbac/rbca_test.go b/server/service/rbac/rbca_test.go
index 6c2596f..c72b884 100644
--- a/server/service/rbac/rbca_test.go
+++ b/server/service/rbac/rbca_test.go
@@ -91,7 +91,7 @@ func TestInitRBAC(t *testing.T) {
 	t.Run("change self password", func(t *testing.T) {
 		err := dao.CreateAccount(context.Background(), &rbacframe.Account{Name: "b", Password: "Complicated_password1"})
 		assert.NoError(t, err)
-		err = rbac.ChangePassword(context.Background(), "", "b", &rbacframe.Account{CurrentPassword: "Complicated_password1", Password: "Complicated_password2"})
+		err = rbac.ChangePassword(context.Background(), "", "b", &rbacframe.Account{Name: "b", CurrentPassword: "Complicated_password1", Password: "Complicated_password2"})
 		assert.NoError(t, err)
 		a, err := dao.GetAccount(context.Background(), "b")
 		assert.NoError(t, err)
diff --git a/server/service/validate.go b/server/service/validate.go
index 1c8e10c..05c679b 100644
--- a/server/service/validate.go
+++ b/server/service/validate.go
@@ -37,8 +37,8 @@ func init() {
 	createAccountValidator.AddRule("Role", &validate.Rule{Regexp: roleRegex})
 	createAccountValidator.AddRule("Password", &validate.Rule{Regexp: &validate.PasswordChecker{}})
 
-	changePWDValidator.AddRule("CurrentPassword", &validate.Rule{Min: 8})
 	changePWDValidator.AddRule("Password", &validate.Rule{Regexp: &validate.PasswordChecker{}})
+	changePWDValidator.AddRule("Name", &validate.Rule{Regexp: accountRegex})
 }
 func Validate(v interface{}) error {
 	err := baseCheck(v)