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/05/22 08:07:23 UTC
[servicecomb-service-center] branch master updated: Realize all
rbac APIs. (#1000)
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 <hu...@163.com>
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{}})