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 2021/06/02 03:12:44 UTC

[servicecomb-service-center] branch master updated: Add new rbac error code (#1032)

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 5e15a9c  Add new rbac error code (#1032)
5e15a9c is described below

commit 5e15a9c2d9f02b3555d4a0c24cd11af4cf1d2934
Author: humingcheng <hu...@163.com>
AuthorDate: Wed Jun 2 11:12:35 2021 +0800

    Add new rbac error code (#1032)
    
    * Add new rbac error code
    
    * Add account status validator
---
 go.mod                                             |  2 +-
 go.sum                                             |  2 +
 server/resource/v4/auth_resource.go                | 51 ++-------------
 server/resource/v4/role_resource.go                | 30 ++++-----
 server/service/rbac/account_dao.go                 | 23 ++++---
 server/service/rbac/account_dao_test.go            | 16 ++---
 server/service/rbac/authr_plugin.go                | 51 +++++++++------
 server/service/rbac/blocker.go                     |  1 +
 server/service/rbac/password.go                    | 65 +++++++++++--------
 server/service/rbac/rbac.go                        |  5 ++
 server/service/rbac/rbac_test.go                   | 16 ++++-
 server/service/rbac/resource.go                    |  1 -
 server/service/rbac/role.go                        | 17 ++---
 server/service/rbac/role_dao.go                    | 75 +++++++++++-----------
 server/service/rbac/role_dao_test.go               | 63 +++++++-----------
 server/service/validator/microservice_validator.go |  2 +
 server/service/validator/rbac_validator.go         |  8 +++
 server/service/validator/rbac_validator_test.go    | 46 +++++++++++++
 server/service/validator/validator.go              |  7 +-
 19 files changed, 260 insertions(+), 221 deletions(-)

diff --git a/go.mod b/go.mod
index fa01301..0dff043 100644
--- a/go.mod
+++ b/go.mod
@@ -15,7 +15,7 @@ require (
 	github.com/dgrijalva/jwt-go v3.2.0+incompatible
 	github.com/elithrar/simple-scrypt v1.3.0
 	github.com/ghodss/yaml v1.0.0
-	github.com/go-chassis/cari v0.4.1-0.20210530055018-eabe8a37291c
+	github.com/go-chassis/cari v0.4.1-0.20210601163026-bb6a506e336a
 	github.com/go-chassis/foundation v0.3.1-0.20210513015331-b54416b66bcd
 	github.com/go-chassis/go-archaius v1.5.1
 	github.com/go-chassis/go-chassis/v2 v2.1.2-0.20210310004133-c9bc42149a18
diff --git a/go.sum b/go.sum
index 71e5f9d..3c1c0aa 100644
--- a/go.sum
+++ b/go.sum
@@ -280,6 +280,8 @@ github.com/go-chassis/cari v0.4.1-0.20210528093055-1c7737622daf h1:g0Q9IJr+jrmPO
 github.com/go-chassis/cari v0.4.1-0.20210528093055-1c7737622daf/go.mod h1:av/19fqwEP4eOC8unL/z67AAbFDwXUCko6SKa4Avrd8=
 github.com/go-chassis/cari v0.4.1-0.20210530055018-eabe8a37291c h1:F00R3MGfNXTZMToWEfjQ4k/uJ3Si5gpoQ3GyfcAdHnA=
 github.com/go-chassis/cari v0.4.1-0.20210530055018-eabe8a37291c/go.mod h1:av/19fqwEP4eOC8unL/z67AAbFDwXUCko6SKa4Avrd8=
+github.com/go-chassis/cari v0.4.1-0.20210601163026-bb6a506e336a h1:vIG+7IEYeeswrPK8/SM6rDK312M7/7N7sLlYcJ1j6AQ=
+github.com/go-chassis/cari v0.4.1-0.20210601163026-bb6a506e336a/go.mod h1:av/19fqwEP4eOC8unL/z67AAbFDwXUCko6SKa4Avrd8=
 github.com/go-chassis/foundation v0.2.2-0.20201210043510-9f6d3de40234/go.mod h1:2PjwqpVwYEVaAldl5A58a08viH8p27pNeYaiE3ZxOBA=
 github.com/go-chassis/foundation v0.2.2/go.mod h1:2PjwqpVwYEVaAldl5A58a08viH8p27pNeYaiE3ZxOBA=
 github.com/go-chassis/foundation v0.3.0/go.mod h1:2PjwqpVwYEVaAldl5A58a08viH8p27pNeYaiE3ZxOBA=
diff --git a/server/resource/v4/auth_resource.go b/server/resource/v4/auth_resource.go
index 1c92440..6b3042f 100644
--- a/server/resource/v4/auth_resource.go
+++ b/server/resource/v4/auth_resource.go
@@ -19,7 +19,6 @@ package v4
 
 import (
 	"encoding/json"
-	"fmt"
 	"github.com/go-chassis/cari/pkg/errsvc"
 	"io/ioutil"
 	"net/http"
@@ -27,7 +26,6 @@ import (
 	errorsEx "github.com/apache/servicecomb-service-center/pkg/errors"
 	"github.com/apache/servicecomb-service-center/pkg/log"
 	"github.com/apache/servicecomb-service-center/pkg/rest"
-	"github.com/apache/servicecomb-service-center/pkg/util"
 	rbacsvc "github.com/apache/servicecomb-service-center/server/service/rbac"
 	"github.com/apache/servicecomb-service-center/server/service/validator"
 
@@ -135,13 +133,6 @@ func (ar *AuthResource) GetAccount(w http.ResponseWriter, r *http.Request) {
 }
 
 func (ar *AuthResource) ChangePassword(w http.ResponseWriter, req *http.Request) {
-	name := req.URL.Query().Get(":name")
-	ip := util.GetRealIP(req)
-	if rbacsvc.IsBanned(MakeBanKey(name, ip)) {
-		log.Warn(fmt.Sprintf("ip is banned:%s, account: %s", ip, req.URL.Query().Get(":name")))
-		rest.WriteError(w, discovery.ErrForbidden, "")
-		return
-	}
 	body, err := ioutil.ReadAll(req.Body)
 	if err != nil {
 		log.Error("read body err", err)
@@ -155,32 +146,10 @@ func (ar *AuthResource) ChangePassword(w http.ResponseWriter, req *http.Request)
 		return
 	}
 	a.Name = req.URL.Query().Get(":name")
-	err = validator.ValidateChangePWD(a)
-	if err != nil {
-		rest.WriteError(w, discovery.ErrInvalidParams, err.Error())
-		return
-	}
 
-	changer, err := rbacsvc.AccountFromContext(req.Context())
-	if err != nil {
-		rest.WriteError(w, discovery.ErrInternal, "can not parse account info")
-		return
-	}
-	err = rbacsvc.ChangePassword(req.Context(), changer.Roles, changer.Name, a)
+	err = rbacsvc.ChangePassword(req.Context(), a)
 	if err != nil {
-		if err == rbacsvc.ErrSamePassword ||
-			err == rbacsvc.ErrEmptyCurrentPassword ||
-			err == rbacsvc.ErrNoPermChangeAccount {
-			rest.WriteError(w, discovery.ErrInvalidParams, err.Error())
-			return
-		}
-		if err == rbacsvc.ErrWrongPassword {
-			rbacsvc.CountFailure(MakeBanKey(a.Name, ip))
-			rest.WriteError(w, discovery.ErrInvalidParams, err.Error())
-			return
-		}
-		log.Error("change password failed", err)
-		rest.WriteError(w, discovery.ErrInternal, err.Error())
+		writeErrsvcOrInternalErr(w, err)
 		return
 	}
 	rest.WriteSuccess(w, req)
@@ -207,23 +176,11 @@ func (ar *AuthResource) Login(w http.ResponseWriter, r *http.Request) {
 		rest.WriteError(w, discovery.ErrInvalidParams, err.Error())
 		return
 	}
-	ip := util.GetRealIP(r)
-	if rbacsvc.IsBanned(MakeBanKey(a.Name, ip)) {
-		log.Warn(fmt.Sprintf("ip is banned:%s, account: %s", ip, a.Name))
-		rest.WriteError(w, discovery.ErrForbidden, "")
-		return
-	}
 	t, err := authr.Login(r.Context(), a.Name, a.Password,
 		authr.ExpireAfter(a.TokenExpirationTime))
 	if err != nil {
-		if err == rbacsvc.ErrUnauthorized {
-			log.Error("not authorized", err)
-			rbacsvc.CountFailure(MakeBanKey(a.Name, ip))
-			rest.WriteError(w, rbac.ErrUserOrPwdWrong, err.Error())
-			return
-		}
-		log.Error("can not sign token", err)
-		rest.WriteError(w, discovery.ErrInternal, err.Error())
+		log.Error("not authorized", err)
+		writeErrsvcOrInternalErr(w, err)
 		return
 	}
 	rest.WriteResponse(w, r, nil, &rbac.Token{TokenStr: t})
diff --git a/server/resource/v4/role_resource.go b/server/resource/v4/role_resource.go
index a2debe8..2c92ce2 100644
--- a/server/resource/v4/role_resource.go
+++ b/server/resource/v4/role_resource.go
@@ -19,8 +19,6 @@ package v4
 
 import (
 	"encoding/json"
-	"errors"
-	"github.com/apache/servicecomb-service-center/datasource"
 	rbacsvc "github.com/apache/servicecomb-service-center/server/service/rbac"
 	"io/ioutil"
 	"net/http"
@@ -90,13 +88,13 @@ func (rr *RoleResource) CreateRole(w http.ResponseWriter, req *http.Request) {
 		return
 	}
 
-	status, err := rbacsvc.CreateRole(req.Context(), role)
+	err = rbacsvc.CreateRole(req.Context(), role)
 	if err != nil {
 		log.Error(errorsEx.MsgOperateRoleFailed, err)
-		rest.WriteError(w, discovery.ErrInternal, err.Error())
+		writeErrsvcOrInternalErr(w, err)
 		return
 	}
-	rest.WriteResponse(w, req, status, nil)
+	rest.WriteResponse(w, req, nil, nil)
 }
 
 //UpdateRole update role permissions
@@ -113,42 +111,38 @@ func (rr *RoleResource) UpdateRole(w http.ResponseWriter, req *http.Request) {
 		rest.WriteError(w, discovery.ErrInvalidParams, errorsEx.MsgJSON)
 		return
 	}
-	status, err := rbacsvc.EditRole(req.Context(), name, role)
+	err = rbacsvc.EditRole(req.Context(), name, role)
 	if err != nil {
 		log.Error(errorsEx.MsgOperateRoleFailed, err)
-		rest.WriteError(w, discovery.ErrInternal, errorsEx.MsgOperateRoleFailed)
+		writeErrsvcOrInternalErr(w, err)
 		return
 	}
 
-	rest.WriteResponse(w, req, status, nil)
+	rest.WriteResponse(w, req, nil, nil)
 }
 
 //GetRole get the role info according to role name
 func (rr *RoleResource) GetRole(w http.ResponseWriter, r *http.Request) {
-	resp, status, err := rbacsvc.GetRole(r.Context(), r.URL.Query().Get(":roleName"))
+	resp, err := rbacsvc.GetRole(r.Context(), r.URL.Query().Get(":roleName"))
 	if err != nil {
 		log.Error(errorsEx.MsgGetRoleFailed, err)
-		rest.WriteError(w, discovery.ErrInternal, errorsEx.MsgGetRoleFailed)
+		writeErrsvcOrInternalErr(w, err)
 		return
 	}
 
-	rest.WriteResponse(w, r, status, resp)
+	rest.WriteResponse(w, r, nil, resp)
 }
 
 //DeleteRole delete the role info by role name
 func (rr *RoleResource) DeleteRole(w http.ResponseWriter, req *http.Request) {
 	n := req.URL.Query().Get(":roleName")
 
-	status, err := rbacsvc.DeleteRole(req.Context(), n)
-	if errors.Is(err, datasource.ErrRoleBindingExist) {
-		rest.WriteError(w, discovery.ErrInvalidParams, errorsEx.MsgJSON)
-		return
-	}
+	err := rbacsvc.DeleteRole(req.Context(), n)
 	if err != nil {
 		log.Error(errorsEx.MsgJSON, err)
-		rest.WriteError(w, discovery.ErrInternal, errorsEx.MsgJSON)
+		writeErrsvcOrInternalErr(w, err)
 		return
 	}
 
-	rest.WriteResponse(w, req, status, nil)
+	rest.WriteResponse(w, req, nil, nil)
 }
diff --git a/server/service/rbac/account_dao.go b/server/service/rbac/account_dao.go
index cb800b0..befd20b 100644
--- a/server/service/rbac/account_dao.go
+++ b/server/service/rbac/account_dao.go
@@ -42,12 +42,15 @@ func CreateAccount(ctx context.Context, a *rbac.Account) error {
 	err := validator.ValidateCreateAccount(a)
 	if err != nil {
 		log.Errorf(err, "create account [%s] failed", a.Name)
-		return rbac.NewError(discovery.ErrInvalidParams, err.Error())
+		return discovery.NewError(discovery.ErrInvalidParams, err.Error())
+	}
+	if len(a.Status) == 0 {
+		a.Status = "active"
 	}
 	err = a.Check()
 	if err != nil {
 		log.Errorf(err, "create account [%s] failed", a.Name)
-		return rbac.NewError(discovery.ErrInvalidParams, err.Error())
+		return discovery.NewError(discovery.ErrInvalidParams, err.Error())
 	}
 	if err = checkRoleNames(ctx, a.Roles); err != nil {
 		return rbac.NewError(rbac.ErrAccountHasInvalidRole, err.Error())
@@ -68,11 +71,15 @@ func CreateAccount(ctx context.Context, a *rbac.Account) error {
 // UpdateAccount updates an account's info, except the password
 func UpdateAccount(ctx context.Context, name string, a *rbac.Account) error {
 	// todo params validation
-	if err := illegalCheck(ctx, name); err != nil {
+	err := validator.ValidateUpdateAccount(a)
+	if err != nil {
+		return discovery.NewError(discovery.ErrInvalidParams, err.Error())
+	}
+	if err = illegalAccountCheck(ctx, name); err != nil {
 		return err
 	}
 	if len(a.Status) == 0 && len(a.Roles) == 0 {
-		return rbac.NewError(discovery.ErrInvalidParams, "status and roles cannot be empty both")
+		return discovery.NewError(discovery.ErrInvalidParams, "status and roles cannot be empty both")
 	}
 
 	oldAccount, err := GetAccount(ctx, name)
@@ -116,7 +123,7 @@ func AccountExist(ctx context.Context, name string) (bool, error) {
 	return datasource.Instance().AccountExist(ctx, name)
 }
 func DeleteAccount(ctx context.Context, name string) error {
-	if err := illegalCheck(ctx, name); err != nil {
+	if err := illegalAccountCheck(ctx, name); err != nil {
 		return err
 	}
 	exist, err := datasource.Instance().AccountExist(ctx, name)
@@ -167,13 +174,13 @@ func checkRoleNames(ctx context.Context, roles []string) error {
 	return nil
 }
 
-func illegalCheck(ctx context.Context, target string) error {
+func illegalAccountCheck(ctx context.Context, target string) error {
 	if target == RootName {
-		return discovery.NewError(discovery.ErrForbidden, errorsEx.MsgCantOperateRoot)
+		return rbac.NewError(rbac.ErrForbidOperateBuildInAccount, errorsEx.MsgCantOperateRoot)
 	}
 	changer := UserFromContext(ctx)
 	if target == changer {
-		return discovery.NewError(discovery.ErrForbidden, errorsEx.MsgCantOperateYour)
+		return rbac.NewError(rbac.ErrForbidOperateSelfAccount, errorsEx.MsgCantOperateYour)
 	}
 	return nil
 }
diff --git a/server/service/rbac/account_dao_test.go b/server/service/rbac/account_dao_test.go
index 546c307..a7a4df7 100644
--- a/server/service/rbac/account_dao_test.go
+++ b/server/service/rbac/account_dao_test.go
@@ -99,13 +99,13 @@ func TestDeleteAccount(t *testing.T) {
 		svcErr := err.(*errsvc.Error)
 		assert.Equal(t, rbac.ErrAccountNotExist, svcErr.Code)
 	})
-	t.Run("delete root, should return: "+rbac.NewError(discovery.ErrForbidden, "").Error(), func(t *testing.T) {
+	t.Run("delete root, should return: "+rbac.NewError(rbac.ErrForbidOperateBuildInAccount, "").Error(), func(t *testing.T) {
 		err := rbacsvc.DeleteAccount(context.TODO(), "root")
 		assert.NotNil(t, err)
 		svcErr := err.(*errsvc.Error)
-		assert.Equal(t, discovery.ErrForbidden, svcErr.Code)
+		assert.Equal(t, rbac.ErrForbidOperateBuildInAccount, svcErr.Code)
 	})
-	t.Run("delete self, should return: "+rbac.NewError(discovery.ErrForbidden, "").Error(), func(t *testing.T) {
+	t.Run("delete self, should return: "+rbac.NewError(rbac.ErrForbidOperateSelfAccount, "").Error(), func(t *testing.T) {
 		a := newAccount("TestDeleteAccount_delete_self")
 		err := rbacsvc.CreateAccount(context.TODO(), a)
 		assert.Nil(t, err)
@@ -116,7 +116,7 @@ func TestDeleteAccount(t *testing.T) {
 		err = rbacsvc.DeleteAccount(ctx, a.Name)
 		assert.NotNil(t, err)
 		svcErr := err.(*errsvc.Error)
-		assert.Equal(t, discovery.ErrForbidden, svcErr.Code)
+		assert.Equal(t, rbac.ErrForbidOperateSelfAccount, svcErr.Code)
 	})
 }
 
@@ -143,12 +143,12 @@ func TestUpdateAccount(t *testing.T) {
 		svcErr := err.(*errsvc.Error)
 		assert.Equal(t, rbac.ErrAccountNotExist, svcErr.Code)
 	})
-	t.Run("update root, should return: "+discovery.NewError(discovery.ErrForbidden, "").Error(), func(t *testing.T) {
+	t.Run("update root, should return: "+discovery.NewError(rbac.ErrForbidOperateBuildInAccount, "").Error(), func(t *testing.T) {
 		a := newAccount("root")
 		err := rbacsvc.UpdateAccount(context.TODO(), a.Name, a)
 		assert.NotNil(t, err)
 		svcErr := err.(*errsvc.Error)
-		assert.Equal(t, discovery.ErrForbidden, svcErr.Code)
+		assert.Equal(t, rbac.ErrForbidOperateBuildInAccount, svcErr.Code)
 	})
 	t.Run("account has invalid role, should return: "+rbac.NewError(rbac.ErrAccountHasInvalidRole, "").Error(), func(t *testing.T) {
 		name := "TestUpdateAccount_account_has_invalid_role"
@@ -176,7 +176,7 @@ func TestUpdateAccount(t *testing.T) {
 		svcErr := err.(*errsvc.Error)
 		assert.Equal(t, discovery.ErrInvalidParams, svcErr.Code)
 	})
-	t.Run("update self, should return: "+rbac.NewError(discovery.ErrForbidden, "").Error(), func(t *testing.T) {
+	t.Run("update self, should return: "+rbac.NewError(rbac.ErrForbidOperateSelfAccount, "").Error(), func(t *testing.T) {
 		name := "TestDeleteAccount_update_self"
 		a := newAccount(name)
 		err := rbacsvc.CreateAccount(context.TODO(), a)
@@ -190,7 +190,7 @@ func TestUpdateAccount(t *testing.T) {
 		err = rbacsvc.UpdateAccount(ctx, a.Name, a)
 		assert.NotNil(t, err)
 		svcErr := err.(*errsvc.Error)
-		assert.Equal(t, discovery.ErrForbidden, svcErr.Code)
+		assert.Equal(t, rbac.ErrForbidOperateSelfAccount, svcErr.Code)
 	})
 }
 
diff --git a/server/service/rbac/authr_plugin.go b/server/service/rbac/authr_plugin.go
index b6a4615..5d90d41 100644
--- a/server/service/rbac/authr_plugin.go
+++ b/server/service/rbac/authr_plugin.go
@@ -23,6 +23,8 @@ import (
 	"errors"
 	"fmt"
 	"github.com/apache/servicecomb-service-center/datasource"
+	"github.com/apache/servicecomb-service-center/pkg/util"
+	"github.com/go-chassis/cari/pkg/errsvc"
 	"github.com/go-chassis/cari/rbac"
 
 	"github.com/dgrijalva/jwt-go"
@@ -45,36 +47,45 @@ func newEmbeddedAuthenticator(opts *authr.Options) (authr.Authenticator, error)
 
 //Login check db user and password,will verify and return token for valid account
 func (a *EmbeddedAuthenticator) Login(ctx context.Context, user string, password string, opts ...authr.LoginOption) (string, error) {
+	ip := util.GetIPFromContext(ctx)
+	if IsBanned(MakeBanKey(user, ip)) {
+		log.Warnf("ip [%s] is banned, account: %s", ip, user)
+		return "", rbac.NewError(rbac.ErrAccountBlocked, "")
+	}
 	opt := &authr.LoginOptions{}
 	for _, o := range opts {
 		o(opt)
 	}
 	account, err := GetAccount(ctx, user)
 	if err != nil {
-		log.Error("get account err", err)
+		if errsvc.IsErrEqualCode(err, rbac.ErrAccountNotExist) {
+			CountFailure(MakeBanKey(user, ip))
+			return "", rbac.NewError(rbac.ErrUserOrPwdWrong, "")
+		}
 		return "", err
 	}
-
 	same := privacy.SamePassword(account.Password, password)
-	if user == account.Name && same {
-		secret, err := GetPrivateKey()
-		if err != nil {
-			return "", err
-		}
-		tokenStr, err := token.Sign(map[string]interface{}{
-			rbac.ClaimsUser:  user,
-			rbac.ClaimsRoles: account.Roles,
-		},
-			secret,
-			token.WithExpTime(opt.ExpireAfter),
-			token.WithSigningMethod(token.RS512)) //TODO config for each user
-		if err != nil {
-			log.Errorf(err, "can not sign a token")
-			return "", err
-		}
-		return tokenStr, nil
+	if !same {
+		CountFailure(MakeBanKey(user, ip))
+		return "", rbac.NewError(rbac.ErrUserOrPwdWrong, "")
+	}
+
+	secret, err := GetPrivateKey()
+	if err != nil {
+		return "", err
+	}
+	tokenStr, err := token.Sign(map[string]interface{}{
+		rbac.ClaimsUser:  user,
+		rbac.ClaimsRoles: account.Roles,
+	},
+		secret,
+		token.WithExpTime(opt.ExpireAfter),
+		token.WithSigningMethod(token.RS512)) //TODO config for each user
+	if err != nil {
+		log.Errorf(err, "can not sign a token")
+		return "", err
 	}
-	return "", ErrUnauthorized
+	return tokenStr, nil
 }
 
 //Authenticate parse a token to claims
diff --git a/server/service/rbac/blocker.go b/server/service/rbac/blocker.go
index dce5053..9060b39 100644
--- a/server/service/rbac/blocker.go
+++ b/server/service/rbac/blocker.go
@@ -99,3 +99,4 @@ func IsBanned(key string) bool {
 	}
 	return client.Banned
 }
+
diff --git a/server/service/rbac/password.go b/server/service/rbac/password.go
index aaf4e21..61d540e 100644
--- a/server/service/rbac/password.go
+++ b/server/service/rbac/password.go
@@ -19,9 +19,11 @@ package rbac
 
 import (
 	"context"
-
 	"github.com/apache/servicecomb-service-center/pkg/privacy"
-	rbacmodel "github.com/go-chassis/cari/rbac"
+	"github.com/apache/servicecomb-service-center/pkg/util"
+	"github.com/apache/servicecomb-service-center/server/service/validator"
+	"github.com/go-chassis/cari/discovery"
+	"github.com/go-chassis/cari/rbac"
 
 	"github.com/go-chassis/foundation/stringutil"
 	"golang.org/x/crypto/bcrypt"
@@ -29,23 +31,32 @@ import (
 	"github.com/apache/servicecomb-service-center/pkg/log"
 )
 
-func ChangePassword(ctx context.Context, changerRole []string, changerName string, a *rbacmodel.Account) error {
-	if changerName == a.Name {
-		if a.CurrentPassword == "" {
-			log.Error("current pwd is empty", nil)
-			return ErrEmptyCurrentPassword
-		}
-		return changePassword(ctx, changerName, a.CurrentPassword, a.Password)
+func ChangePassword(ctx context.Context, a *rbac.Account) error {
+	err := validator.ValidateChangePWD(a)
+	if err != nil {
+		return discovery.NewError(discovery.ErrInvalidParams, err.Error())
 	}
-	for i := 0; i < len(changerRole); i++ {
-		//need to check password mismatch. but admin role can change any user password without supply current password
-		if changerRole[i] == rbacmodel.RoleAdmin {
+
+	changer, err := AccountFromContext(ctx)
+	if err != nil {
+		return discovery.NewError(discovery.ErrInternal, err.Error())
+	}
+
+	// authority has been checked before
+	// admin role can change any user password without supply current password
+	for _, r := range changer.Roles {
+		if r == rbac.RoleAdmin {
 			return changePasswordForcibly(ctx, a.Name, a.Password)
 		}
 	}
-
-	log.Error("can not change other account pwd", nil)
-	return ErrNoPermChangeAccount
+	// change self password, or user with account authority(not admin role)
+	// change other one's password
+	// need to check password mismatch
+	if a.CurrentPassword == "" {
+		log.Error("current pwd is empty", nil)
+		return discovery.NewError(discovery.ErrInvalidParams, ErrEmptyCurrentPassword.Error())
+	}
+	return changePassword(ctx, a.Name, a.CurrentPassword, a.Password)
 }
 func changePasswordForcibly(ctx context.Context, name, pwd string) error {
 	old, err := GetAccount(ctx, name)
@@ -53,15 +64,16 @@ func changePasswordForcibly(ctx context.Context, name, pwd string) error {
 		log.Error("can not change pwd", err)
 		return err
 	}
-	err = doChangePassword(ctx, old, pwd)
-	if err != nil {
-		return err
-	}
-	return nil
+	return doChangePassword(ctx, old, pwd)
 }
 func changePassword(ctx context.Context, name, currentPassword, pwd string) error {
+	ip := util.GetIPFromContext(ctx)
+	if IsBanned(MakeBanKey(name, ip)) {
+		log.Warnf("ip [%s] is banned, account: %s", ip, name)
+		return rbac.NewError(rbac.ErrAccountBlocked, "")
+	}
 	if currentPassword == pwd {
-		return ErrSamePassword
+		return rbac.NewError(rbac.ErrNewPwdBad, ErrSamePassword.Error())
 	}
 	old, err := GetAccount(ctx, name)
 	if err != nil {
@@ -71,16 +83,13 @@ func changePassword(ctx context.Context, name, currentPassword, pwd string) erro
 	same := privacy.SamePassword(old.Password, currentPassword)
 	if !same {
 		log.Error("current password is wrong", nil)
-		return ErrWrongPassword
+		CountFailure(MakeBanKey(name, ip))
+		return rbac.NewError(rbac.ErrOldPwdWrong, "")
 	}
-	err = doChangePassword(ctx, old, pwd)
-	if err != nil {
-		return err
-	}
-	return nil
+	return doChangePassword(ctx, old, pwd)
 }
 
-func doChangePassword(ctx context.Context, old *rbacmodel.Account, pwd string) error {
+func doChangePassword(ctx context.Context, old *rbac.Account, pwd string) error {
 	hash, err := bcrypt.GenerateFromPassword([]byte(pwd), 14)
 	if err != nil {
 		log.Error("pwd hash failed", err)
diff --git a/server/service/rbac/rbac.go b/server/service/rbac/rbac.go
index d72597c..74f59f3 100644
--- a/server/service/rbac/rbac.go
+++ b/server/service/rbac/rbac.go
@@ -174,3 +174,8 @@ func GetPrivateKey() (*rsa.PrivateKey, error) {
 	}
 	return p, nil
 }
+
+//MakeBanKey return ban key
+func MakeBanKey(name, ip string) string {
+	return name + "::" + ip
+}
diff --git a/server/service/rbac/rbac_test.go b/server/service/rbac/rbac_test.go
index fd55df6..195b4c3 100644
--- a/server/service/rbac/rbac_test.go
+++ b/server/service/rbac/rbac_test.go
@@ -81,7 +81,14 @@ func TestInitRBAC(t *testing.T) {
 		persisted := newAccount("admin_change_other_pwd")
 		err := rbacsvc.CreateAccount(context.Background(), persisted)
 		assert.NoError(t, err)
-		err = rbacsvc.ChangePassword(context.Background(), []string{rbac.RoleAdmin}, rbac.RoleAdmin, &rbac.Account{Name: persisted.Name, Password: "Complicated_password2"})
+		context.Background()
+
+		claims := map[string]interface{}{
+			rbac.ClaimsUser:  "test",
+			rbac.ClaimsRoles: []interface{}{rbac.RoleAdmin},
+		}
+		ctx := context.WithValue(context.Background(), rbacsvc.CtxRequestClaims, claims)
+		err = rbacsvc.ChangePassword(ctx, &rbac.Account{Name: persisted.Name, Password: "Complicated_password2"})
 		assert.NoError(t, err)
 		a, err := rbacsvc.GetAccount(context.Background(), persisted.Name)
 		assert.NoError(t, err)
@@ -91,7 +98,12 @@ func TestInitRBAC(t *testing.T) {
 		a := newAccount("change_self_pwd")
 		err := rbacsvc.CreateAccount(context.Background(), a)
 		assert.NoError(t, err)
-		err = rbacsvc.ChangePassword(context.Background(), nil, a.Name, &rbac.Account{Name: a.Name, CurrentPassword: testPwd0, Password: testPwd1})
+		claims := map[string]interface{}{
+			rbac.ClaimsUser:  "change_self_pwd",
+			rbac.ClaimsRoles: []interface{}{rbac.RoleDeveloper},
+		}
+		ctx := context.WithValue(context.Background(), rbacsvc.CtxRequestClaims, claims)
+		err = rbacsvc.ChangePassword(ctx, &rbac.Account{Name: a.Name, CurrentPassword: testPwd0, Password: testPwd1})
 		assert.NoError(t, err)
 		resp, err := rbacsvc.GetAccount(context.Background(), a.Name)
 		assert.NoError(t, err)
diff --git a/server/service/rbac/resource.go b/server/service/rbac/resource.go
index 71917a2..b6155ae 100644
--- a/server/service/rbac/resource.go
+++ b/server/service/rbac/resource.go
@@ -93,5 +93,4 @@ func InitResourceMap() {
 	rbac.MapResource(APIServiceRule, ResourceService)
 	rbac.MapResource(APIServiceTag, ResourceService)
 	rbac.MapResource(APIServiceTagKey, ResourceService)
-
 }
diff --git a/server/service/rbac/role.go b/server/service/rbac/role.go
index 8753706..5b19b6f 100644
--- a/server/service/rbac/role.go
+++ b/server/service/rbac/role.go
@@ -19,9 +19,9 @@ package rbac
 
 import (
 	"context"
-	"github.com/go-chassis/cari/rbac"
-
 	"github.com/apache/servicecomb-service-center/pkg/log"
+	"github.com/go-chassis/cari/pkg/errsvc"
+	"github.com/go-chassis/cari/rbac"
 )
 
 var roleMap = map[string]*rbac.Role{}
@@ -45,18 +45,15 @@ func initBuildInRole() {
 }
 
 func createBuildInRole(r *rbac.Role) {
-	status, err := CreateRole(context.Background(), r)
-	if err != nil {
-		log.Fatalf(err, "create role [%s] failed", r.Name)
-		return
-	}
-	if status.IsSucceed() {
+	err := CreateRole(context.Background(), r)
+	if err == nil {
 		log.Infof("create role [%s] success", r.Name)
 		return
 	}
-	if status.Code == rbac.ErrRoleConflict {
+	if errsvc.IsErrEqualCode(err, rbac.ErrRoleConflict) {
 		log.Infof("role [%s] already exists", r.Name)
 		return
 	}
-	log.Fatalf(nil, "create role [%s] failed: %s", r.Name, status.GetMessage())
+	log.Fatalf(err, "create role [%s] failed", r.Name)
+	return
 }
diff --git a/server/service/rbac/role_dao.go b/server/service/rbac/role_dao.go
index 01896ec..00c022d 100644
--- a/server/service/rbac/role_dao.go
+++ b/server/service/rbac/role_dao.go
@@ -20,52 +20,51 @@ package rbac
 import (
 	"context"
 	"errors"
+	"github.com/apache/servicecomb-service-center/datasource"
 	errorsEx "github.com/apache/servicecomb-service-center/pkg/errors"
+	"github.com/apache/servicecomb-service-center/pkg/log"
 	"github.com/apache/servicecomb-service-center/pkg/util"
 	"github.com/apache/servicecomb-service-center/server/plugin/quota"
 	"github.com/apache/servicecomb-service-center/server/service/validator"
 	"github.com/go-chassis/cari/discovery"
 
-	"github.com/apache/servicecomb-service-center/datasource"
-	"github.com/apache/servicecomb-service-center/pkg/log"
-
 	"github.com/go-chassis/cari/rbac"
 )
 
-func CreateRole(ctx context.Context, r *rbac.Role) (*discovery.Response, error) {
+func CreateRole(ctx context.Context, r *rbac.Role) error {
 	err := validator.ValidateCreateRole(r)
 	if err != nil {
 		log.Errorf(err, "create role [%s] failed", r.Name)
-		return discovery.CreateResponse(discovery.ErrInvalidParams, err.Error()), nil
+		return discovery.NewError(discovery.ErrInvalidParams, err.Error())
 	}
 	quotaErr := quota.Apply(ctx, quota.NewApplyQuotaResource(quota.TypeRole,
 		util.ParseDomainProject(ctx), "", 1))
 	if quotaErr != nil {
-		return discovery.CreateResponse(rbac.ErrRoleNoQuota, quotaErr.Error()), nil
+		return rbac.NewError(rbac.ErrRoleNoQuota, quotaErr.Error())
 	}
 	err = datasource.Instance().CreateRole(ctx, r)
 	if err == nil {
 		log.Infof("create role [%s] success", r.Name)
-		return nil, nil
+		return nil
 	}
 
 	log.Errorf(err, "create role [%s] failed", r.Name)
 	if err == datasource.ErrRoleDuplicated {
-		return discovery.CreateResponse(rbac.ErrRoleConflict, err.Error()), nil
+		return rbac.NewError(rbac.ErrRoleConflict, err.Error())
 	}
 
-	return nil, err
+	return err
 }
 
-func GetRole(ctx context.Context, name string) (*rbac.Role, *discovery.Response, error) {
+func GetRole(ctx context.Context, name string) (*rbac.Role, error) {
 	resp, err := datasource.Instance().GetRole(ctx, name)
 	if err == nil {
-		return resp, nil, nil
+		return resp, nil
 	}
 	if err == datasource.ErrRoleNotExist {
-		return nil, discovery.CreateResponse(rbac.ErrRoleNotExist, ""), nil
+		return nil, rbac.NewError(rbac.ErrRoleNotExist, "")
 	}
-	return nil, nil, err
+	return nil, err
 }
 
 func ListRole(ctx context.Context) ([]*rbac.Role, int64, error) {
@@ -76,49 +75,49 @@ func RoleExist(ctx context.Context, name string) (bool, error) {
 	return datasource.Instance().RoleExist(ctx, name)
 }
 
-func DeleteRole(ctx context.Context, name string) (*discovery.Response, error) {
-	if isBuildInRole(name) {
-		return discovery.CreateResponse(discovery.ErrForbidden, errorsEx.MsgCantOperateBuildInRole), nil
+func DeleteRole(ctx context.Context, name string) error {
+	if err := illegalRoleCheck(name); err != nil {
+		return err
 	}
-	exist, err := datasource.Instance().RoleExist(ctx, name)
+	exist, err := RoleExist(ctx, name)
 	if err != nil {
 		log.Errorf(err, "check role [%s] exist failed", name)
-		return nil, err
+		return err
 	}
 	if !exist {
 		log.Errorf(err, "role [%s] not exist", name)
-		return discovery.CreateResponse(rbac.ErrRoleNotExist, ""), nil
+		return rbac.NewError(rbac.ErrRoleNotExist, "")
 	}
 	succeed, err := datasource.Instance().DeleteRole(ctx, name)
 	if err != nil {
-		return nil, err
+		if errors.Is(err, datasource.ErrRoleBindingExist) {
+			return rbac.NewError(rbac.ErrRoleIsBound, "")
+		}
+		return err
 	}
 	if !succeed {
-		return nil, errors.New("delete role failed, please retry")
+		return errors.New("delete role failed, please retry")
 	}
-	return nil, nil
+	return nil
 }
 
-func EditRole(ctx context.Context, name string, a *rbac.Role) (*discovery.Response, error) {
-	if isBuildInRole(name) {
-		return discovery.CreateResponse(discovery.ErrForbidden, errorsEx.MsgCantOperateBuildInRole), nil
+func EditRole(ctx context.Context, name string, a *rbac.Role) error {
+	if err := illegalRoleCheck(name); err != nil {
+		return err
 	}
-	exist, err := datasource.Instance().RoleExist(ctx, name)
+	exist, err := RoleExist(ctx, name)
 	if err != nil {
 		log.Errorf(err, "check role [%s] exist failed", name)
-		return nil, err
+		return err
 	}
 	if !exist {
 		log.Errorf(err, "role [%s] not exist", name)
-		return discovery.CreateResponse(rbac.ErrRoleNotExist, ""), nil
+		return rbac.NewError(rbac.ErrRoleNotExist, "")
 	}
-	oldRole, status, err := GetRole(ctx, name)
+	oldRole, err := GetRole(ctx, name)
 	if err != nil {
 		log.Errorf(err, "get role [%s] failed", name)
-		return nil, err
-	}
-	if status != nil && status.GetCode() != discovery.ResponseSuccess {
-		return status, nil
+		return err
 	}
 
 	oldRole.Perms = a.Perms
@@ -126,15 +125,15 @@ func EditRole(ctx context.Context, name string, a *rbac.Role) (*discovery.Respon
 	err = datasource.Instance().UpdateRole(ctx, name, oldRole)
 	if err != nil {
 		log.Errorf(err, "can not edit role info")
-		return nil, err
+		return err
 	}
 	log.Infof("role [%s] is edit", oldRole.ID)
-	return nil, nil
+	return nil
 }
 
-func isBuildInRole(role string) bool {
+func illegalRoleCheck(role string) error {
 	if role == rbac.RoleAdmin || role == rbac.RoleDeveloper {
-		return true
+		return rbac.NewError(rbac.ErrForbidOperateBuildInRole, errorsEx.MsgCantOperateBuildInRole)
 	}
-	return false
+	return nil
 }
diff --git a/server/service/rbac/role_dao_test.go b/server/service/rbac/role_dao_test.go
index 995a576..18d3bc8 100644
--- a/server/service/rbac/role_dao_test.go
+++ b/server/service/rbac/role_dao_test.go
@@ -3,7 +3,7 @@ package rbac_test
 import (
 	"context"
 	rbacsvc "github.com/apache/servicecomb-service-center/server/service/rbac"
-	"github.com/go-chassis/cari/discovery"
+	"github.com/go-chassis/cari/pkg/errsvc"
 	"github.com/go-chassis/cari/rbac"
 	"github.com/stretchr/testify/assert"
 	"testing"
@@ -28,37 +28,31 @@ func newRole(name string) *rbac.Role {
 func TestCreateRole(t *testing.T) {
 	t.Run("create new role, should succeed", func(t *testing.T) {
 		r := newRole("TestCreateRole_createNewRole")
-		status, err := rbacsvc.CreateRole(context.TODO(), r)
+		err := rbacsvc.CreateRole(context.TODO(), r)
 		assert.Nil(t, err)
-		assert.True(t, status.IsSucceed())
 	})
 	t.Run("create role twice, should return: "+rbac.NewError(rbac.ErrRoleConflict, "").Error(), func(t *testing.T) {
 		r := newRole("TestCreateRole_createRoleTwice")
-		status, err := rbacsvc.CreateRole(context.TODO(), r)
+		err := rbacsvc.CreateRole(context.TODO(), r)
 		assert.Nil(t, err)
-		assert.True(t, status.IsSucceed())
 		// twice
-		status, err = rbacsvc.CreateRole(context.TODO(), r)
-		assert.Nil(t, err)
-		assert.Equal(t, rbac.ErrRoleConflict, status.GetCode())
+		err = rbacsvc.CreateRole(context.TODO(), r)
+		assert.True(t, errsvc.IsErrEqualCode(err, rbac.ErrRoleConflict))
 	})
 }
 
 func TestGetRole(t *testing.T) {
 	t.Run("get no exist role, should return: "+rbac.NewError(rbac.ErrRoleNotExist, "").Error(), func(t *testing.T) {
-		r, status, err := rbacsvc.GetRole(context.TODO(), "TestGetRole_getNoExistRole")
-		assert.Nil(t, err)
-		assert.Equal(t, rbac.ErrRoleNotExist, status.GetCode())
+		r, err := rbacsvc.GetRole(context.TODO(), "TestGetRole_getNoExistRole")
+		assert.True(t, errsvc.IsErrEqualCode(err, rbac.ErrRoleNotExist))
 		assert.Nil(t, r)
 	})
 	t.Run("get exist role, should success", func(t *testing.T) {
 		r := newRole("TestGetRole_getExistRole")
-		status, err := rbacsvc.CreateRole(context.TODO(), r)
+		err := rbacsvc.CreateRole(context.TODO(), r)
 		assert.Nil(t, err)
-		assert.True(t, status.IsSucceed())
-		resp, status, err := rbacsvc.GetRole(context.TODO(), r.Name)
+		resp, err := rbacsvc.GetRole(context.TODO(), r.Name)
 		assert.Nil(t, err)
-		assert.True(t, status.IsSucceed())
 		assert.Equal(t, r.Name, resp.Name)
 	})
 }
@@ -66,15 +60,13 @@ func TestGetRole(t *testing.T) {
 func TestEditRole(t *testing.T) {
 	t.Run("edit no exist role, should return: "+rbac.NewError(rbac.ErrRoleNotExist, "").Error(), func(t *testing.T) {
 		r := newRole("TestEditRole_editNoExistRole")
-		status, err := rbacsvc.EditRole(context.TODO(), r.Name, r)
-		assert.Nil(t, err)
-		assert.Equal(t, rbac.ErrRoleNotExist, status.GetCode())
+		err := rbacsvc.EditRole(context.TODO(), r.Name, r)
+		assert.True(t, errsvc.IsErrEqualCode(err, rbac.ErrRoleNotExist))
 	})
 	t.Run("edit role, should success", func(t *testing.T) {
 		r := newRole("TestGetRole_editRole")
-		status, err := rbacsvc.CreateRole(context.TODO(), r)
+		err := rbacsvc.CreateRole(context.TODO(), r)
 		assert.Nil(t, err)
-		assert.True(t, status.IsSucceed())
 
 		// edit
 		assert.Equal(t, 1, len(r.Perms))
@@ -96,49 +88,42 @@ func TestEditRole(t *testing.T) {
 				Verbs: []string{"*"},
 			},
 		}
-		status, err = rbacsvc.EditRole(context.TODO(), r.Name, r)
+		err = rbacsvc.EditRole(context.TODO(), r.Name, r)
 		assert.Nil(t, err)
-		assert.True(t, status.IsSucceed())
 
-		resp, status, err := rbacsvc.GetRole(context.TODO(), r.Name)
+		resp, err := rbacsvc.GetRole(context.TODO(), r.Name)
 		assert.Nil(t, err)
-		assert.True(t, status.IsSucceed())
 		assert.Equal(t, 2, len(resp.Perms))
 	})
-	t.Run("edit build in role, should return: "+discovery.NewError(discovery.ErrForbidden, "").Error(), func(t *testing.T) {
+	t.Run("edit build in role, should return: "+rbac.NewError(rbac.ErrForbidOperateBuildInRole, "").Error(), func(t *testing.T) {
 		for _, name := range []string{rbac.RoleDeveloper, rbac.RoleDeveloper} {
-			status, err := rbacsvc.EditRole(context.TODO(), name, newRole(""))
-			assert.Nil(t, err)
-			assert.Equal(t, discovery.ErrForbidden, status.GetCode())
+			err := rbacsvc.EditRole(context.TODO(), name, newRole(""))
+			assert.True(t, errsvc.IsErrEqualCode(err, rbac.ErrForbidOperateBuildInRole))
 		}
 	})
 }
 
 func TestDeleteRole(t *testing.T) {
 	t.Run("delete no exist role, should return: "+rbac.NewError(rbac.ErrRoleNotExist, "").Error(), func(t *testing.T) {
-		status, err := rbacsvc.DeleteRole(context.TODO(), "TestDeleteRole_deleteNoExistRole")
-		assert.Nil(t, err)
-		assert.Equal(t, rbac.ErrRoleNotExist, status.GetCode())
+		err := rbacsvc.DeleteRole(context.TODO(), "TestDeleteRole_deleteNoExistRole")
+		assert.True(t, errsvc.IsErrEqualCode(err, rbac.ErrRoleNotExist))
 	})
 	t.Run("delete role, should success", func(t *testing.T) {
 		r := newRole("TestDeleteRole_deleteRole")
-		status, err := rbacsvc.CreateRole(context.TODO(), r)
+		err := rbacsvc.CreateRole(context.TODO(), r)
 		assert.Nil(t, err)
-		assert.True(t, status.IsSucceed())
 
-		status, err = rbacsvc.DeleteRole(context.TODO(), r.Name)
+		err = rbacsvc.DeleteRole(context.TODO(), r.Name)
 		assert.Nil(t, err)
-		assert.True(t, status.IsSucceed())
 
 		exist, err := rbacsvc.RoleExist(context.TODO(), r.Name)
 		assert.Nil(t, err)
 		assert.False(t, exist)
 	})
-	t.Run("delete build in role, should return: "+discovery.NewError(discovery.ErrForbidden, "").Error(), func(t *testing.T) {
+	t.Run("delete build in role, should return: "+rbac.NewError(rbac.ErrForbidOperateBuildInRole, "").Error(), func(t *testing.T) {
 		for _, name := range []string{rbac.RoleDeveloper, rbac.RoleDeveloper} {
-			status, err := rbacsvc.DeleteRole(context.TODO(), name)
-			assert.Nil(t, err)
-			assert.Equal(t, discovery.ErrForbidden, status.GetCode())
+			err := rbacsvc.DeleteRole(context.TODO(), name)
+			assert.True(t, errsvc.IsErrEqualCode(err, rbac.ErrForbidOperateBuildInRole))
 		}
 	})
 }
diff --git a/server/service/validator/microservice_validator.go b/server/service/validator/microservice_validator.go
index 44d4b53..d02f170 100644
--- a/server/service/validator/microservice_validator.go
+++ b/server/service/validator/microservice_validator.go
@@ -52,6 +52,8 @@ var (
 	envRegex, _        = regexp.Compile("^(" + util.StringJoin([]string{
 		discovery.ENV_DEV, discovery.ENV_TEST, discovery.ENV_ACCEPT, discovery.ENV_PROD}, "|") + ")*$")
 	schemaIDRegex, _ = regexp.Compile(`^[a-zA-Z0-9]{1,160}$|^[a-zA-Z0-9][a-zA-Z0-9_\-.]{0,158}[a-zA-Z0-9]$`)
+
+	accountStatusRegex, _ = regexp.Compile(`^(active|inactive)$|^$`)
 )
 
 func MicroServiceKeyValidator() *validate.Validator {
diff --git a/server/service/validator/rbac_validator.go b/server/service/validator/rbac_validator.go
index d4497a5..a88680a 100644
--- a/server/service/validator/rbac_validator.go
+++ b/server/service/validator/rbac_validator.go
@@ -9,6 +9,14 @@ func ValidateCreateAccount(a *rbac.Account) error {
 	}
 	return createAccountValidator.Validate(a)
 }
+
+func ValidateUpdateAccount(a *rbac.Account) error {
+	err := baseCheck(a)
+	if err != nil {
+		return err
+	}
+	return updateAccountValidator.Validate(a)
+}
 func ValidateCreateRole(a *rbac.Role) error {
 	err := baseCheck(a)
 	if err != nil {
diff --git a/server/service/validator/rbac_validator_test.go b/server/service/validator/rbac_validator_test.go
index 6523f94..47b2ff2 100644
--- a/server/service/validator/rbac_validator_test.go
+++ b/server/service/validator/rbac_validator_test.go
@@ -18,6 +18,7 @@
 package validator_test
 
 import (
+	"github.com/stretchr/testify/assert"
 	"testing"
 
 	"github.com/apache/servicecomb-service-center/server/service/validator"
@@ -71,6 +72,51 @@ func TestValidateCreateAccount(t *testing.T) {
 			t.Errorf("%q. ValidateCreateAccount() error = %v, wantErr %v", tt.name, err, tt.wantErr)
 		}
 	}
+
+	a := &rbac.Account{
+		Name:     "tester",
+		Password: "Pwd0000_1",
+		Roles:    []string{"admin", "developer"},
+	}
+	assert.NoError(t, validator.ValidateCreateAccount(a))
+
+	a.Roles = []string{"admin", "developer", "test1", "test1", "test3", "test4"}
+	assert.Error(t, validator.ValidateCreateAccount(a))
+
+	a.Roles = []string{}
+	assert.Error(t, validator.ValidateCreateAccount(a))
+
+	a.Roles = []string{"admin"}
+	a.Status = "active"
+	assert.NoError(t, validator.ValidateCreateAccount(a))
+
+	a.Status = "active1"
+	assert.Error(t, validator.ValidateCreateAccount(a))
+}
+
+func TestValidateUpdateAccount(t *testing.T) {
+	a := &rbac.Account{
+		Roles: []string{"admin", "developer"},
+	}
+	assert.NoError(t, validator.ValidateUpdateAccount(a))
+
+	a = &rbac.Account{
+		Roles: []string{"admin", "developer", "test1", "test1", "test3"},
+	}
+	assert.NoError(t, validator.ValidateUpdateAccount(a))
+
+	a.Roles = []string{"admin", "developer", "test1", "test1", "test3", "test4"}
+	assert.Error(t, validator.ValidateUpdateAccount(a))
+
+	a.Roles = []string{}
+	assert.Error(t, validator.ValidateUpdateAccount(a))
+
+	a.Roles = []string{"admin"}
+	a.Status = "active"
+	assert.NoError(t, validator.ValidateUpdateAccount(a))
+
+	a.Status = "active1"
+	assert.Error(t, validator.ValidateUpdateAccount(a))
 }
 
 func TestValidateCreateRole(t *testing.T) {
diff --git a/server/service/validator/validator.go b/server/service/validator/validator.go
index 994e1bf..04392b5 100644
--- a/server/service/validator/validator.go
+++ b/server/service/validator/validator.go
@@ -26,6 +26,7 @@ import (
 )
 
 var createAccountValidator = &validate.Validator{}
+var updateAccountValidator = &validate.Validator{}
 var createRoleValidator = &validate.Validator{}
 
 var changePWDValidator = &validate.Validator{}
@@ -33,8 +34,12 @@ var accountLoginValidator = &validate.Validator{}
 
 func init() {
 	createAccountValidator.AddRule("Name", &validate.Rule{Max: 64, Regexp: nameRegex})
-	createAccountValidator.AddRule("Roles", &validate.Rule{Min: 1, Regexp: nameRegex})
+	createAccountValidator.AddRule("Roles", &validate.Rule{Min: 1, Max: 5, Regexp: nameRegex})
 	createAccountValidator.AddRule("Password", &validate.Rule{Regexp: &validate.PasswordChecker{}})
+	createAccountValidator.AddRule("Status", &validate.Rule{Regexp: accountStatusRegex})
+
+	updateAccountValidator.AddRule("Roles", createAccountValidator.GetRule("Roles"))
+	updateAccountValidator.AddRule("Status", createAccountValidator.GetRule("Status"))
 
 	createRoleValidator.AddRule("Name", &validate.Rule{Max: 64, Regexp: nameRegex})