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 5f84c2c  Realize all rbac APIs. (#1000)
5f84c2c is described below

commit 5f84c2cf01a514cb6c9329aa086d6ec7f9beca5a
Author: humingcheng <[email protected]>
AuthorDate: Sat May 22 16:07:15 2021 +0800

    Realize all rbac APIs. (#1000)
    
    * Realize all rbac APIs.
    
    * Add UT and logs
---
 datasource/account.go                       |  1 +
 docs/openapi/v4.yaml                        |  1 +
 pkg/rbacframe/api.go                        |  3 +-
 server/resource/v4/auth_resource.go         | 32 ++++++++++++++++--
 server/resource/v4/rbac_resource_test.go    | 12 ++++---
 server/resource/v4/role_resource.go         | 10 ++++--
 server/rest/controller/rest_util.go         |  9 ++++++
 server/service/rbac/dao/account_dao.go      | 41 +++++++++++++++++++++--
 server/service/rbac/dao/account_dao_test.go | 50 +++++++++++++++++++++++++----
 server/service/rbac/dao/role_dao.go         | 20 ++++++++----
 server/service/rbac/rbac.go                 |  4 +--
 server/service/rbac/rbac_test.go            |  2 +-
 server/service/validate.go                  |  2 +-
 13 files changed, 155 insertions(+), 32 deletions(-)

diff --git a/datasource/account.go b/datasource/account.go
index 2e706a0..0c66730 100644
--- a/datasource/account.go
+++ b/datasource/account.go
@@ -30,6 +30,7 @@ var (
        ErrDLockNotFound       = errors.New("dlock not found")
        ErrDeleteAccountFailed = errors.New("failed to delete account")
        ErrQueryAccountFailed  = errors.New("failed to query account")
+       ErrAccountNotExist     = errors.New("account not exist")
 )
 
 // AccountManager contains the RBAC CRUD
diff --git a/docs/openapi/v4.yaml b/docs/openapi/v4.yaml
index 59e9465..5551c18 100644
--- a/docs/openapi/v4.yaml
+++ b/docs/openapi/v4.yaml
@@ -2965,6 +2965,7 @@ definitions:
     required:
       - password
       - name
+      - roles
     properties:
       id:
         type: string
diff --git a/pkg/rbacframe/api.go b/pkg/rbacframe/api.go
index 57b8846..49a74b0 100644
--- a/pkg/rbacframe/api.go
+++ b/pkg/rbacframe/api.go
@@ -32,7 +32,8 @@ const (
        ClaimsUser  = "account"
        ClaimsRoles = "roles"
 
-       RoleAdmin = "admin"
+       RoleAdmin     = "admin"
+       RoleDeveloper = "developer"
 )
 
 var whiteAPIList = sets.NewString()
diff --git a/server/resource/v4/auth_resource.go 
b/server/resource/v4/auth_resource.go
index ab54e1f..c1d3b41 100644
--- a/server/resource/v4/auth_resource.go
+++ b/server/resource/v4/auth_resource.go
@@ -23,8 +23,6 @@ import (
        "io/ioutil"
        "net/http"
 
-       rbacsvc 
"github.com/apache/servicecomb-service-center/server/service/rbac"
-
        "github.com/apache/servicecomb-service-center/datasource"
        errorsEx "github.com/apache/servicecomb-service-center/pkg/errors"
        "github.com/apache/servicecomb-service-center/pkg/log"
@@ -32,7 +30,9 @@ import (
        "github.com/apache/servicecomb-service-center/pkg/util"
        "github.com/apache/servicecomb-service-center/server/rest/controller"
        "github.com/apache/servicecomb-service-center/server/service"
+       rbacsvc 
"github.com/apache/servicecomb-service-center/server/service/rbac"
        "github.com/apache/servicecomb-service-center/server/service/rbac/dao"
+
        "github.com/go-chassis/cari/discovery"
        "github.com/go-chassis/cari/rbac"
        "github.com/go-chassis/go-chassis/v2/security/authr"
@@ -49,6 +49,7 @@ func (r *AuthResource) URLPatterns() []rest.Route {
                {Method: http.MethodGet, Path: "/v4/accounts", Func: 
r.ListAccount},
                {Method: http.MethodGet, Path: "/v4/accounts/:name", Func: 
r.GetAccount},
                {Method: http.MethodDelete, Path: "/v4/accounts/:name", Func: 
r.DeleteAccount},
+               {Method: http.MethodPut, Path: "/v4/accounts/:name", Func: 
r.UpdateAccount},
                {Method: http.MethodPost, Path: "/v4/accounts/:name/password", 
Func: r.ChangePassword},
        }
 }
@@ -83,6 +84,7 @@ func (r *AuthResource) CreateAccount(w http.ResponseWriter, 
req *http.Request) {
                controller.WriteError(w, discovery.ErrInternal, 
errorsEx.MsgOperateAccountFailed)
                return
        }
+       controller.WriteSuccess(w, req)
 }
 func (r *AuthResource) DeleteAccount(w http.ResponseWriter, req *http.Request) 
{
        _, err := dao.DeleteAccount(context.TODO(), 
req.URL.Query().Get(":name"))
@@ -91,7 +93,29 @@ func (r *AuthResource) DeleteAccount(w http.ResponseWriter, 
req *http.Request) {
                controller.WriteError(w, discovery.ErrInternal, 
errorsEx.MsgOperateAccountFailed)
                return
        }
-       w.WriteHeader(http.StatusNoContent)
+       controller.WriteSuccess(w, req)
+}
+func (r *AuthResource) UpdateAccount(w http.ResponseWriter, req *http.Request) 
{
+       body, err := ioutil.ReadAll(req.Body)
+       if err != nil {
+               log.Error("read body err", err)
+               controller.WriteError(w, discovery.ErrInternal, err.Error())
+               return
+       }
+       a := &rbac.Account{}
+       if err = json.Unmarshal(body, a); err != nil {
+               log.Error("json err", err)
+               controller.WriteError(w, discovery.ErrInvalidParams, 
err.Error())
+               return
+       }
+       name := req.URL.Query().Get(":name")
+       err = dao.UpdateAccount(context.TODO(), name, a)
+       if err != nil {
+               log.Error(errorsEx.MsgOperateAccountFailed, err)
+               controller.WriteError(w, discovery.ErrInternal, 
errorsEx.MsgOperateAccountFailed)
+               return
+       }
+       controller.WriteSuccess(w, req)
 }
 func (r *AuthResource) ListAccount(w http.ResponseWriter, req *http.Request) {
        as, n, err := dao.ListAccount(context.TODO())
@@ -128,6 +152,7 @@ func (r *AuthResource) GetAccount(w http.ResponseWriter, 
req *http.Request) {
        }
        controller.WriteJSON(w, b)
 }
+
 func (r *AuthResource) ChangePassword(w http.ResponseWriter, req 
*http.Request) {
        ip := util.GetRealIP(req)
        if rbacsvc.IsBanned(ip) {
@@ -175,6 +200,7 @@ func (r *AuthResource) ChangePassword(w 
http.ResponseWriter, req *http.Request)
                controller.WriteError(w, discovery.ErrInternal, err.Error())
                return
        }
+       controller.WriteSuccess(w, req)
 }
 func (r *AuthResource) Login(w http.ResponseWriter, req *http.Request) {
        ip := util.GetRealIP(req)
diff --git a/server/resource/v4/rbac_resource_test.go 
b/server/resource/v4/rbac_resource_test.go
index 7e01a10..4418157 100644
--- a/server/resource/v4/rbac_resource_test.go
+++ b/server/resource/v4/rbac_resource_test.go
@@ -29,6 +29,7 @@ import (
 
        rbacmodel "github.com/go-chassis/cari/rbac"
 
+       "github.com/apache/servicecomb-service-center/pkg/rbacframe"
        "github.com/apache/servicecomb-service-center/pkg/rest"
        "github.com/apache/servicecomb-service-center/server/config"
        v4 "github.com/apache/servicecomb-service-center/server/resource/v4"
@@ -165,15 +166,18 @@ func TestAuthResource_DeleteAccount(t *testing.T) {
                assert.Equal(t, http.StatusUnauthorized, w2.Code)
        })
        t.Run("root can delete account", func(t *testing.T) {
-               b, _ := json.Marshal(&rbacmodel.Account{Name: "root", Password: 
"Complicated_password1"})
-               r, _ := http.NewRequest(http.MethodPost, "/v4/token", 
bytes.NewBuffer(b))
+               var err error
+               b, err := json.Marshal(&rbacmodel.Account{Name: "root", 
Password: "Complicated_password1"})
+               assert.NoError(t, err)
+               r, err := http.NewRequest(http.MethodPost, "/v4/token", 
bytes.NewBuffer(b))
+               assert.NoError(t, err)
                w := httptest.NewRecorder()
                rest.GetRouter().ServeHTTP(w, r)
                assert.Equal(t, http.StatusOK, w.Code)
                to := &rbacmodel.Token{}
                json.Unmarshal(w.Body.Bytes(), to)
 
-               b, _ = json.Marshal(&rbacmodel.Account{Name: "delete_account", 
Password: "Complicated_password1"})
+               b, _ = json.Marshal(&rbacmodel.Account{Name: "delete_account", 
Password: "Complicated_password1", Roles: []string{rbacframe.RoleDeveloper}})
                r2, _ := http.NewRequest(http.MethodPost, "/v4/accounts", 
bytes.NewBuffer(b))
                r2.Header.Set(restful.HeaderAuth, "Bearer "+to.TokenStr)
                w2 := httptest.NewRecorder()
@@ -184,7 +188,7 @@ func TestAuthResource_DeleteAccount(t *testing.T) {
                r3.Header.Set(restful.HeaderAuth, "Bearer "+to.TokenStr)
                w3 := httptest.NewRecorder()
                rest.GetRouter().ServeHTTP(w3, r3)
-               assert.Equal(t, http.StatusNoContent, w3.Code)
+               assert.Equal(t, http.StatusOK, w3.Code)
        })
 }
 func TestAuthResource_GetAccount(t *testing.T) {
diff --git a/server/resource/v4/role_resource.go 
b/server/resource/v4/role_resource.go
index 57e6867..fd73c27 100644
--- a/server/resource/v4/role_resource.go
+++ b/server/resource/v4/role_resource.go
@@ -23,15 +23,15 @@ import (
        "io/ioutil"
        "net/http"
 
-       "github.com/go-chassis/cari/rbac"
-
        "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/rest"
        "github.com/apache/servicecomb-service-center/server/rest/controller"
        "github.com/apache/servicecomb-service-center/server/service/rbac/dao"
+
        "github.com/go-chassis/cari/discovery"
+       "github.com/go-chassis/cari/rbac"
 )
 
 var ErrConflictRole int32 = 409002
@@ -105,6 +105,7 @@ func (r *RoleResource) CreateRolePermission(w 
http.ResponseWriter, req *http.Req
                controller.WriteError(w, discovery.ErrInternal, 
errorsEx.MsgOperateRoleFailed)
                return
        }
+       controller.WriteSuccess(w, req)
 }
 
 //UpdateRolePermission update role permissions
@@ -120,12 +121,14 @@ func (r *RoleResource) UpdateRolePermission(w 
http.ResponseWriter, req *http.Req
                controller.WriteError(w, discovery.ErrInvalidParams, 
errorsEx.MsgJSON)
                return
        }
-       err = dao.EditRole(context.TODO(), role)
+       name := req.URL.Query().Get(":roleName")
+       err = dao.EditRole(context.TODO(), name, role)
        if err != nil {
                log.Error(errorsEx.MsgOperateRoleFailed, err)
                controller.WriteError(w, discovery.ErrInternal, 
errorsEx.MsgOperateRoleFailed)
                return
        }
+       controller.WriteSuccess(w, req)
 }
 
 //GetRole get the role info according to role name
@@ -152,4 +155,5 @@ func (r *RoleResource) DeleteRole(w http.ResponseWriter, 
req *http.Request) {
                controller.WriteError(w, discovery.ErrInternal, 
errorsEx.MsgJSON)
                return
        }
+       controller.WriteSuccess(w, req)
 }
diff --git a/server/rest/controller/rest_util.go 
b/server/rest/controller/rest_util.go
index c1698c7..2c77835 100644
--- a/server/rest/controller/rest_util.go
+++ b/server/rest/controller/rest_util.go
@@ -45,6 +45,11 @@ func WriteError(w http.ResponseWriter, code int32, detail 
string) {
        }
 }
 
+// WriteResponse writes http response
+// If the resp is nil or represents success, response status is http.StatusOK,
+// response content is obj.
+// If the resp represents fail, response status is from the code in the
+// resp, response content is from the message in the resp.
 func WriteResponse(w http.ResponseWriter, r *http.Request, resp 
*discovery.Response, obj interface{}) {
        if resp != nil && resp.GetCode() != discovery.ResponseSuccess {
                WriteError(w, resp.GetCode(), resp.GetMessage())
@@ -90,3 +95,7 @@ func WriteJSON(w http.ResponseWriter, json []byte) {
                log.Error("", err)
        }
 }
+
+func WriteSuccess(w http.ResponseWriter, r *http.Request) {
+       WriteResponse(w, r, nil, nil)
+}
diff --git a/server/service/rbac/dao/account_dao.go 
b/server/service/rbac/dao/account_dao.go
index 80ca08e..2c6b8dc 100644
--- a/server/service/rbac/dao/account_dao.go
+++ b/server/service/rbac/dao/account_dao.go
@@ -20,11 +20,12 @@ package dao
 
 import (
        "context"
-
-       rbacmodel "github.com/go-chassis/cari/rbac"
+       "errors"
 
        "github.com/apache/servicecomb-service-center/datasource"
        "github.com/apache/servicecomb-service-center/pkg/log"
+
+       rbacmodel "github.com/go-chassis/cari/rbac"
 )
 
 //CreateAccount save 2 kv
@@ -33,6 +34,40 @@ func CreateAccount(ctx context.Context, a 
*rbacmodel.Account) error {
        return datasource.Instance().CreateAccount(ctx, a)
 }
 
+// UpdateAccount updates an account's info, except the password
+func UpdateAccount(ctx context.Context, name string, a *rbacmodel.Account) 
error {
+       // todo params validation
+       if len(a.Status) == 0 && len(a.Roles) == 0 {
+               return errors.New("status and roles cannot be empty both")
+       }
+       exist, err := datasource.Instance().AccountExist(ctx, name)
+       if err != nil {
+               log.Errorf(err, "check account [%s] exit failed", name)
+               return err
+       }
+       if !exist {
+               return datasource.ErrAccountNotExist
+       }
+       oldAccount, err := GetAccount(ctx, name)
+       if err != nil {
+               log.Errorf(err, "get account [%s] failed", name)
+               return err
+       }
+       if len(a.Status) != 0 {
+               oldAccount.Status = a.Status
+       }
+       if len(a.Roles) != 0 {
+               oldAccount.Roles = a.Roles
+       }
+       err = datasource.Instance().UpdateAccount(ctx, name, oldAccount)
+       if err != nil {
+               log.Errorf(err, "can not edit account info")
+               return err
+       }
+       log.Infof("account [%s] is edit", oldAccount.ID)
+       return nil
+}
+
 func GetAccount(ctx context.Context, name string) (*rbacmodel.Account, error) {
        return datasource.Instance().GetAccount(ctx, name)
 }
@@ -63,6 +98,6 @@ func EditAccount(ctx context.Context, a *rbacmodel.Account) 
error {
                log.Errorf(err, "can not edit account info")
                return err
        }
-       log.Info("account is edit")
+       log.Infof("account [%s] is edit", a.ID)
        return nil
 }
diff --git a/server/service/rbac/dao/account_dao_test.go 
b/server/service/rbac/dao/account_dao_test.go
index a3e6a2d..04437eb 100644
--- a/server/service/rbac/dao/account_dao_test.go
+++ b/server/service/rbac/dao/account_dao_test.go
@@ -20,12 +20,14 @@ package dao_test
 // initialize
 import (
        "context"
-       "github.com/go-chassis/cari/rbac"
        "testing"
 
+       "github.com/apache/servicecomb-service-center/pkg/rbacframe"
        "github.com/apache/servicecomb-service-center/server/service/rbac/dao"
        _ "github.com/apache/servicecomb-service-center/test"
+
        "github.com/astaxie/beego"
+       "github.com/go-chassis/cari/rbac"
        "github.com/stretchr/testify/assert"
        "golang.org/x/crypto/bcrypt"
 )
@@ -33,15 +35,49 @@ import (
 func init() {
        beego.AppConfig.Set("registry_plugin", "etcd")
 }
+
+func newAccount(name string) *rbac.Account {
+       return &rbac.Account{
+               Name:     name,
+               Password: "Ab@11111",
+               Roles:    []string{rbacframe.RoleAdmin},
+       }
+}
+
 func TestAccountDao_CreateAccount(t *testing.T) {
-       dao.DeleteAccount(context.TODO(), "admin")
-       _ = dao.CreateAccount(context.Background(), &rbac.Account{Name: 
"admin", Password: "pwd"})
+       account := newAccount("createAccountTest")
+       dao.DeleteAccount(context.TODO(), account.Name)
+       _ = dao.CreateAccount(context.Background(), account)
        t.Run("get account", func(t *testing.T) {
-               r, err := dao.GetAccount(context.Background(), "admin")
+               r, err := dao.GetAccount(context.Background(), account.Name)
+               assert.NoError(t, err)
+               assert.Equal(t, account.Name, r.Name)
+               hash, err := 
bcrypt.GenerateFromPassword([]byte(account.Password), 14)
+               err = bcrypt.CompareHashAndPassword(hash, 
[]byte(account.Password))
+               assert.NoError(t, err)
+       })
+}
+func TestAccountDao_UpdateAccount(t *testing.T) {
+       account := newAccount("updateAccountTest")
+       t.Run("update an none exist account", func(t *testing.T) {
+               newAccount := &rbac.Account{Roles: []string{"admin"}}
+               err := dao.UpdateAccount(context.Background(), "noExist", 
newAccount)
+               assert.Error(t, err)
+       })
+
+       dao.DeleteAccount(context.TODO(), account.Name)
+       err := dao.CreateAccount(context.Background(), account)
+       assert.NoError(t, err)
+
+       t.Run("update account", func(t *testing.T) {
+               newAccount := &rbac.Account{
+                       Roles: []string{rbacframe.RoleDeveloper},
+               }
+               err = dao.UpdateAccount(context.Background(), account.Name, 
newAccount)
                assert.NoError(t, err)
-               assert.Equal(t, "admin", r.Name)
-               hash, err := bcrypt.GenerateFromPassword([]byte("pwd"), 14)
-               err = bcrypt.CompareHashAndPassword(hash, []byte("pwd"))
+               a, err := dao.GetAccount(context.Background(), account.Name)
                assert.NoError(t, err)
+               assert.Equal(t, 1, len(a.Roles))
+               assert.Equal(t, rbacframe.RoleDeveloper, a.Roles[0])
        })
 }
diff --git a/server/service/rbac/dao/role_dao.go 
b/server/service/rbac/dao/role_dao.go
index 45feed4..23508de 100644
--- a/server/service/rbac/dao/role_dao.go
+++ b/server/service/rbac/dao/role_dao.go
@@ -20,10 +20,10 @@ package dao
 import (
        "context"
 
-       "github.com/go-chassis/cari/rbac"
-
        "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) error {
@@ -50,21 +50,27 @@ func DeleteRole(ctx context.Context, name string) (bool, 
error) {
        return datasource.Instance().DeleteRole(ctx, name)
 }
 
-func EditRole(ctx context.Context, a *rbac.Role) error {
-       exist, err := datasource.Instance().RoleExist(ctx, a.Name)
+func EditRole(ctx context.Context, name string, a *rbac.Role) error {
+       exist, err := datasource.Instance().RoleExist(ctx, name)
        if err != nil {
-               log.Errorf(err, "can not edit account info")
+               log.Errorf(err, "check role [%s] exist failed", name)
                return err
        }
        if !exist {
                return datasource.ErrRoleCanNotEdit
        }
+       oldRole, err := GetRole(ctx, name)
+       if err != nil {
+               log.Errorf(err, "get role [%s] failed", name)
+               return err
+       }
+       oldRole.Perms = a.Perms
 
-       err = datasource.Instance().UpdateRole(ctx, a.Name, a)
+       err = datasource.Instance().UpdateRole(ctx, name, oldRole)
        if err != nil {
                log.Errorf(err, "can not edit role info")
                return err
        }
-       log.Info("role is edit")
+       log.Infof("role [%s] is edit", oldRole.ID)
        return nil
 }
diff --git a/server/service/rbac/rbac.go b/server/service/rbac/rbac.go
index 07f15ad..d40f9d6 100644
--- a/server/service/rbac/rbac.go
+++ b/server/service/rbac/rbac.go
@@ -77,7 +77,7 @@ func Init() {
        log.Info("rbac is enabled")
 }
 
-//readPublicKey read key to memory
+//read key to memory
 func readPrivateKey() {
        pf := config.GetString("rbac.privateKeyFile", "", 
config.WithStandby("rbac_rsa_private_key_file"))
        // 打开文件
@@ -94,7 +94,7 @@ func readPrivateKey() {
        log.Info("read private key success")
 }
 
-//readPublicKey read key to memory
+//read key to memory
 func readPublicKey() {
        pf := config.GetString("rbac.publicKeyFile", "", 
config.WithStandby("rbac_rsa_public_key_file"))
        // 打开文件
diff --git a/server/service/rbac/rbac_test.go b/server/service/rbac/rbac_test.go
index cb63543..38b03c3 100644
--- a/server/service/rbac/rbac_test.go
+++ b/server/service/rbac/rbac_test.go
@@ -175,7 +175,7 @@ func TestInitRBAC(t *testing.T) {
        })
 
        t.Run("edit new role success", func(t *testing.T) {
-               err := dao.EditRole(context.Background(), tester)
+               err := dao.EditRole(context.Background(), tester.Name, tester)
                assert.NoError(t, err)
 
                r, err := dao.GetRole(context.Background(), "tester")
diff --git a/server/service/validate.go b/server/service/validate.go
index c91c9f8..812cd7c 100644
--- a/server/service/validate.go
+++ b/server/service/validate.go
@@ -36,7 +36,7 @@ func init() {
        var roleRegex, _ = 
regexp.Compile(`^$|^(admin|developer|[a-zA-Z]\w{2,15})$`)
        var accountRegex, _ = regexp.Compile(`^[a-zA-Z]\w{3,15}$`)
        createAccountValidator.AddRule("Name", &validate.Rule{Regexp: 
accountRegex})
-       createAccountValidator.AddRule("Roles", &validate.Rule{Regexp: 
roleRegex})
+       createAccountValidator.AddRule("Roles", &validate.Rule{Min: 1, Regexp: 
roleRegex})
        createAccountValidator.AddRule("Password", &validate.Rule{Regexp: 
&validate.PasswordChecker{}})
 
        changePWDValidator.AddRule("Password", &validate.Rule{Regexp: 
&validate.PasswordChecker{}})

Reply via email to