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/30 13:05:52 UTC
[servicecomb-service-center] branch master updated: Add role manage
error code (#1025)
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 44d5ac8 Add role manage error code (#1025)
44d5ac8 is described below
commit 44d5ac89f3ffe3b9226599ca7f9f5014ff6ee45b
Author: humingcheng <hu...@163.com>
AuthorDate: Sun May 30 21:05:44 2021 +0800
Add role manage error code (#1025)
---
datasource/etcd/role.go | 3 +
datasource/mongo/role.go | 2 +-
datasource/role.go | 1 +
go.mod | 2 +-
go.sum | 2 +
pkg/errors/text.go | 18 ++--
server/handler/auth/auth.go | 6 +-
server/resource/v4/auth_resource.go | 4 +-
server/resource/v4/role_resource.go | 67 +++++--------
server/resource/v4/role_resource_test.go | 2 +-
server/service/rbac/dao/role_dao.go | 89 ++++++++++++++---
server/service/rbac/dao/role_dao_test.go | 162 +++++++++++++++++++++++++++++++
server/service/rbac/decision.go | 16 +--
server/service/rbac/rbac.go | 5 +-
server/service/rbac/rbac_test.go | 96 ++----------------
server/service/rbac/role.go | 40 +++++---
16 files changed, 329 insertions(+), 186 deletions(-)
diff --git a/datasource/etcd/role.go b/datasource/etcd/role.go
index f8f2c47..1cd803c 100644
--- a/datasource/etcd/role.go
+++ b/datasource/etcd/role.go
@@ -88,6 +88,9 @@ func (ds *DataSource) GetRole(ctx context.Context, name string) (*rbac.Role, err
if err != nil {
return nil, err
}
+ if resp.Count == 0 {
+ return nil, datasource.ErrRoleNotExist
+ }
if resp.Count != 1 {
return nil, client.ErrNotUnique
}
diff --git a/datasource/mongo/role.go b/datasource/mongo/role.go
index db75c5d..7ab4ce6 100644
--- a/datasource/mongo/role.go
+++ b/datasource/mongo/role.go
@@ -70,7 +70,7 @@ func (ds *DataSource) GetRole(ctx context.Context, name string) (*rbac.Role, err
return nil, err
}
if result.Err() != nil {
- return nil, client.ErrNoDocuments
+ return nil, datasource.ErrRoleNotExist
}
var role rbac.Role
err = result.Decode(&role)
diff --git a/datasource/role.go b/datasource/role.go
index 74ded86..076f6d6 100644
--- a/datasource/role.go
+++ b/datasource/role.go
@@ -27,6 +27,7 @@ import (
var (
ErrRoleDuplicated = errors.New("role is duplicated")
ErrRoleCanNotEdit = errors.New("role can not be edited")
+ ErrRoleNotExist = errors.New("role not exist")
)
// RoleManager contains the RBAC CRUD
diff --git a/go.mod b/go.mod
index bf99e82..fa01301 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.20210528090900-ebd6bed3bd52
+ github.com/go-chassis/cari v0.4.1-0.20210530055018-eabe8a37291c
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 56318bd..71e5f9d 100644
--- a/go.sum
+++ b/go.sum
@@ -278,6 +278,8 @@ github.com/go-chassis/cari v0.4.1-0.20210528090900-ebd6bed3bd52 h1:ItPO3FBqT6yyW
github.com/go-chassis/cari v0.4.1-0.20210528090900-ebd6bed3bd52/go.mod h1:av/19fqwEP4eOC8unL/z67AAbFDwXUCko6SKa4Avrd8=
github.com/go-chassis/cari v0.4.1-0.20210528093055-1c7737622daf h1:g0Q9IJr+jrmPOzUM6sVNFybenM9xB3psOEgk7MXElTc=
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/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/pkg/errors/text.go b/pkg/errors/text.go
index 5b7e5ea..39b50e2 100644
--- a/pkg/errors/text.go
+++ b/pkg/errors/text.go
@@ -20,13 +20,13 @@ package errors
const (
MsgJSON = "json is invalid"
- MsgOperateAccountFailed = "operate account failed"
- MsgCantOperateRoot = "root can not be operated"
- MsgCantOperateRole = "build-in role can not be operated"
- MsgCantOperateYour = "your account can not be operated"
- MsgOperateRoleFailed = "operate role failed"
- MsgGetAccountFailed = "get account failed"
- MsgGetRoleFailed = "get role failed"
- MsgRolePerm = "check role permissions failed"
- MsgNoPerm = "no permission to operate"
+ MsgOperateAccountFailed = "operate account failed"
+ MsgCantOperateRoot = "root can not be operated"
+ MsgCantOperateBuildInRole = "build-in role can not be operated"
+ MsgCantOperateYour = "your account can not be operated"
+ MsgOperateRoleFailed = "operate role failed"
+ MsgGetAccountFailed = "get account failed"
+ MsgGetRoleFailed = "get role failed"
+ MsgRolePerm = "check role permissions failed"
+ MsgNoPerm = "no permission to operate"
)
diff --git a/server/handler/auth/auth.go b/server/handler/auth/auth.go
index b67c465..641124b 100644
--- a/server/handler/auth/auth.go
+++ b/server/handler/auth/auth.go
@@ -18,6 +18,8 @@
package auth
import (
+ "net/http"
+
"github.com/apache/servicecomb-service-center/pkg/chain"
"github.com/apache/servicecomb-service-center/pkg/log"
"github.com/apache/servicecomb-service-center/pkg/rest"
@@ -25,7 +27,7 @@ import (
"github.com/apache/servicecomb-service-center/server/plugin/auth"
"github.com/apache/servicecomb-service-center/server/response"
"github.com/go-chassis/cari/discovery"
- "net/http"
+ "github.com/go-chassis/cari/rbac"
)
const (
@@ -43,7 +45,7 @@ func (h *Handler) Handle(i *chain.Invocation) {
if err := auth.Identify(r); err != nil {
log.Errorf(err, "authenticate request failed, %s %s", r.Method, r.RequestURI)
- i.Fail(discovery.NewError(discovery.ErrUnauthorized, err.Error()))
+ i.Fail(discovery.NewError(rbac.ErrUnauthorized, err.Error()))
return
}
diff --git a/server/resource/v4/auth_resource.go b/server/resource/v4/auth_resource.go
index 98a5ca4..63536f4 100644
--- a/server/resource/v4/auth_resource.go
+++ b/server/resource/v4/auth_resource.go
@@ -80,7 +80,7 @@ func (ar *AuthResource) CreateAccount(w http.ResponseWriter, req *http.Request)
err = dao.CreateAccount(context.TODO(), a)
if err != nil {
if err == datasource.ErrAccountDuplicated {
- rest.WriteError(w, discovery.ErrConflictAccount, "")
+ rest.WriteError(w, rbac.ErrAccountConflict, "")
return
}
log.Error(errorsEx.MsgOperateAccountFailed, err)
@@ -254,7 +254,7 @@ func (ar *AuthResource) Login(w http.ResponseWriter, r *http.Request) {
if err == rbacsvc.ErrUnauthorized {
log.Error("not authorized", err)
rbacsvc.CountFailure(MakeBanKey(a.Name, ip))
- rest.WriteError(w, discovery.ErrUnauthorized, err.Error())
+ rest.WriteError(w, rbac.ErrUnauthorized, err.Error())
return
}
log.Error("can not sign token", err)
diff --git a/server/resource/v4/role_resource.go b/server/resource/v4/role_resource.go
index 417b690..1a6b089 100644
--- a/server/resource/v4/role_resource.go
+++ b/server/resource/v4/role_resource.go
@@ -20,11 +20,9 @@ package v4
import (
"context"
"encoding/json"
- "github.com/apache/servicecomb-service-center/server/service/validator"
"io/ioutil"
"net/http"
- "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"
@@ -42,16 +40,16 @@ type RoleResource struct {
//URLPatterns define http pattern
func (rr *RoleResource) URLPatterns() []rest.Route {
return []rest.Route{
- {Method: http.MethodGet, Path: "/v4/roles", Func: rr.GetRolePermission},
- {Method: http.MethodPost, Path: "/v4/roles", Func: rr.CreateRolePermission},
- {Method: http.MethodPut, Path: "/v4/roles/:roleName", Func: rr.UpdateRolePermission},
+ {Method: http.MethodGet, Path: "/v4/roles", Func: rr.ListRoles},
+ {Method: http.MethodPost, Path: "/v4/roles", Func: rr.CreateRole},
+ {Method: http.MethodPut, Path: "/v4/roles/:roleName", Func: rr.UpdateRole},
{Method: http.MethodGet, Path: "/v4/roles/:roleName", Func: rr.GetRole},
{Method: http.MethodDelete, Path: "/v4/roles/:roleName", Func: rr.DeleteRole},
}
}
-//GetRolePermission list all roles and there's permissions
-func (rr *RoleResource) GetRolePermission(w http.ResponseWriter, req *http.Request) {
+//ListRoles list all roles and there's permissions
+func (rr *RoleResource) ListRoles(w http.ResponseWriter, req *http.Request) {
rs, num, err := dao.ListRole(context.TODO())
if err != nil {
log.Error(errorsEx.MsgGetRoleFailed, err)
@@ -77,8 +75,8 @@ func (rr *RoleResource) roleParse(body []byte) (*rbac.Role, error) {
return role, nil
}
-//CreateRolePermission create new role and assign permissions
-func (rr *RoleResource) CreateRolePermission(w http.ResponseWriter, req *http.Request) {
+//CreateRole create new role and assign permissions
+func (rr *RoleResource) CreateRole(w http.ResponseWriter, req *http.Request) {
body, err := ioutil.ReadAll(req.Body)
if err != nil {
log.Error("read body err", err)
@@ -90,31 +88,19 @@ func (rr *RoleResource) CreateRolePermission(w http.ResponseWriter, req *http.Re
rest.WriteError(w, discovery.ErrInvalidParams, errorsEx.MsgJSON)
return
}
- err = validator.ValidateCreateRole(role)
- if err != nil {
- rest.WriteError(w, discovery.ErrInvalidParams, err.Error())
- return
- }
- err = dao.CreateRole(context.TODO(), role)
+
+ status, err := dao.CreateRole(context.TODO(), role)
if err != nil {
- if err == datasource.ErrRoleDuplicated {
- rest.WriteError(w, ErrConflictRole, "")
- return
- }
log.Error(errorsEx.MsgOperateRoleFailed, err)
- rest.WriteError(w, discovery.ErrInternal, errorsEx.MsgOperateRoleFailed)
+ rest.WriteError(w, discovery.ErrInternal, err.Error())
return
}
- rest.WriteSuccess(w, req)
+ rest.WriteResponse(w, req, status, nil)
}
-//UpdateRolePermission update role permissions
-func (rr *RoleResource) UpdateRolePermission(w http.ResponseWriter, req *http.Request) {
+//UpdateRole update role permissions
+func (rr *RoleResource) UpdateRole(w http.ResponseWriter, req *http.Request) {
name := req.URL.Query().Get(":roleName")
- if isBuildInRole(name) {
- rest.WriteError(w, discovery.ErrInvalidParams, errorsEx.MsgCantOperateRole)
- return
- }
body, err := ioutil.ReadAll(req.Body)
if err != nil {
log.Error("read body err", err)
@@ -126,43 +112,38 @@ func (rr *RoleResource) UpdateRolePermission(w http.ResponseWriter, req *http.Re
rest.WriteError(w, discovery.ErrInvalidParams, errorsEx.MsgJSON)
return
}
- err = dao.EditRole(context.TODO(), name, role)
+ status, err := dao.EditRole(context.TODO(), name, role)
if err != nil {
log.Error(errorsEx.MsgOperateRoleFailed, err)
rest.WriteError(w, discovery.ErrInternal, errorsEx.MsgOperateRoleFailed)
return
}
- rest.WriteSuccess(w, req)
+
+ rest.WriteResponse(w, req, status, nil)
}
//GetRole get the role info according to role name
func (rr *RoleResource) GetRole(w http.ResponseWriter, r *http.Request) {
- role, err := dao.GetRole(context.TODO(), r.URL.Query().Get(":roleName"))
+ resp, status, err := dao.GetRole(context.TODO(), r.URL.Query().Get(":roleName"))
if err != nil {
log.Error(errorsEx.MsgGetRoleFailed, err)
rest.WriteError(w, discovery.ErrInternal, errorsEx.MsgGetRoleFailed)
+ return
}
- rest.WriteResponse(w, r, nil, role)
+
+ rest.WriteResponse(w, r, status, 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")
- if isBuildInRole(n) {
- rest.WriteError(w, discovery.ErrInvalidParams, errorsEx.MsgCantOperateRole)
- return
- }
- _, err := dao.DeleteRole(context.TODO(), n)
+
+ status, err := dao.DeleteRole(context.TODO(), n)
if err != nil {
log.Error(errorsEx.MsgJSON, err)
rest.WriteError(w, discovery.ErrInternal, errorsEx.MsgJSON)
return
}
- rest.WriteSuccess(w, req)
-}
-func isBuildInRole(role string) bool {
- if role == rbac.RoleAdmin || role == rbac.RoleDeveloper {
- return true
- }
- return false
+
+ rest.WriteResponse(w, req, status, nil)
}
diff --git a/server/resource/v4/role_resource_test.go b/server/resource/v4/role_resource_test.go
index a4939ed..76ea993 100644
--- a/server/resource/v4/role_resource_test.go
+++ b/server/resource/v4/role_resource_test.go
@@ -122,7 +122,7 @@ func TestRoleResource_CreateOrUpdateRole(t *testing.T) {
r3.Header.Set(restful.HeaderAuth, "Bearer "+to.TokenStr)
w3 := httptest.NewRecorder()
rest.GetRouter().ServeHTTP(w3, r3)
- assert.Equal(t, http.StatusBadRequest, w3.Code)
+ assert.Equal(t, http.StatusForbidden, w3.Code)
})
}
func TestRoleResource_MoreRoles(t *testing.T) {
diff --git a/server/service/rbac/dao/role_dao.go b/server/service/rbac/dao/role_dao.go
index 23508de..6964206 100644
--- a/server/service/rbac/dao/role_dao.go
+++ b/server/service/rbac/dao/role_dao.go
@@ -19,6 +19,10 @@ package dao
import (
"context"
+ "errors"
+ errorsEx "github.com/apache/servicecomb-service-center/pkg/errors"
+ "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"
@@ -26,12 +30,35 @@ import (
"github.com/go-chassis/cari/rbac"
)
-func CreateRole(ctx context.Context, r *rbac.Role) error {
- return datasource.Instance().CreateRole(ctx, r)
+func CreateRole(ctx context.Context, r *rbac.Role) (*discovery.Response, 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
+ }
+ err = datasource.Instance().CreateRole(ctx, r)
+ if err == nil {
+ log.Infof("create role [%s] success", r.Name)
+ return nil, nil
+ }
+
+ log.Errorf(err, "create role [%s] failed", r.Name)
+ if err == datasource.ErrRoleDuplicated {
+ return discovery.CreateResponse(rbac.ErrRoleConflict, err.Error()), nil
+ }
+
+ return nil, err
}
-func GetRole(ctx context.Context, name string) (*rbac.Role, error) {
- return datasource.Instance().GetRole(ctx, name)
+func GetRole(ctx context.Context, name string) (*rbac.Role, *discovery.Response, error) {
+ resp, err := datasource.Instance().GetRole(ctx, name)
+ if err == nil {
+ return resp, nil, nil
+ }
+ if err == datasource.ErrRoleNotExist {
+ return nil, discovery.CreateResponse(rbac.ErrRoleNotExist, ""), nil
+ }
+ return nil, nil, err
}
func ListRole(ctx context.Context) ([]*rbac.Role, int64, error) {
@@ -42,35 +69,65 @@ func RoleExist(ctx context.Context, name string) (bool, error) {
return datasource.Instance().RoleExist(ctx, name)
}
-func DeleteRole(ctx context.Context, name string) (bool, error) {
- if name == "admin" || name == "developer" {
- log.Warnf("role %s can not be delete", name)
- return false, nil
+func DeleteRole(ctx context.Context, name string) (*discovery.Response, error) {
+ if isBuildInRole(name) {
+ return discovery.CreateResponse(discovery.ErrForbidden, errorsEx.MsgCantOperateBuildInRole), nil
+ }
+ exist, err := datasource.Instance().RoleExist(ctx, name)
+ if err != nil {
+ log.Errorf(err, "check role [%s] exist failed", name)
+ return nil, err
}
- return datasource.Instance().DeleteRole(ctx, name)
+ if !exist {
+ log.Errorf(err, "role [%s] not exist", name)
+ return discovery.CreateResponse(rbac.ErrRoleNotExist, ""), nil
+ }
+ succeed, err := datasource.Instance().DeleteRole(ctx, name)
+ if err != nil {
+ return nil, err
+ }
+ if !succeed {
+ return nil, errors.New("delete role failed, please retry")
+ }
+ return nil, nil
}
-func EditRole(ctx context.Context, name string, a *rbac.Role) error {
+func EditRole(ctx context.Context, name string, a *rbac.Role) (*discovery.Response, error) {
+ if isBuildInRole(name) {
+ return discovery.CreateResponse(discovery.ErrForbidden, errorsEx.MsgCantOperateBuildInRole), nil
+ }
exist, err := datasource.Instance().RoleExist(ctx, name)
if err != nil {
log.Errorf(err, "check role [%s] exist failed", name)
- return err
+ return nil, err
}
if !exist {
- return datasource.ErrRoleCanNotEdit
+ log.Errorf(err, "role [%s] not exist", name)
+ return discovery.CreateResponse(rbac.ErrRoleNotExist, ""), nil
}
- oldRole, err := GetRole(ctx, name)
+ oldRole, status, err := GetRole(ctx, name)
if err != nil {
log.Errorf(err, "get role [%s] failed", name)
- return err
+ return nil, err
}
+ if status != nil && status.GetCode() != discovery.ResponseSuccess {
+ return status, nil
+ }
+
oldRole.Perms = a.Perms
err = datasource.Instance().UpdateRole(ctx, name, oldRole)
if err != nil {
log.Errorf(err, "can not edit role info")
- return err
+ return nil, err
}
log.Infof("role [%s] is edit", oldRole.ID)
- return nil
+ return nil, nil
+}
+
+func isBuildInRole(role string) bool {
+ if role == rbac.RoleAdmin || role == rbac.RoleDeveloper {
+ return true
+ }
+ return false
}
diff --git a/server/service/rbac/dao/role_dao_test.go b/server/service/rbac/dao/role_dao_test.go
new file mode 100644
index 0000000..443332f
--- /dev/null
+++ b/server/service/rbac/dao/role_dao_test.go
@@ -0,0 +1,162 @@
+package dao_test
+
+import (
+ "context"
+ 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/stretchr/testify/assert"
+ "testing"
+)
+
+func exampleRole(name string) *rbac.Role {
+ return &rbac.Role{
+ Name: name,
+ Perms: []*rbac.Permission{
+ {
+ Resources: []*rbac.Resource{
+ {
+ Type: rbacsvc.ResourceService,
+ },
+ },
+ Verbs: []string{"*"},
+ },
+ },
+ }
+}
+
+func TestCreateRole(t *testing.T) {
+ t.Run("create new role, should succeed", func(t *testing.T) {
+ r := exampleRole("TestCreateRole_createNewRole")
+ status, err := dao.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 := exampleRole("TestCreateRole_createRoleTwice")
+ status, err := dao.CreateRole(context.TODO(), r)
+ assert.Nil(t, err)
+ assert.True(t, status.IsSucceed())
+ // twice
+ status, err = dao.CreateRole(context.TODO(), r)
+ assert.Nil(t, err)
+ assert.Equal(t, rbac.ErrRoleConflict, status.GetCode())
+ })
+}
+
+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 := dao.GetRole(context.TODO(), "TestGetRole_getNoExistRole")
+ assert.Nil(t, err)
+ assert.Equal(t, rbac.ErrRoleNotExist, status.GetCode())
+ assert.Nil(t, r)
+ })
+ t.Run("get exist role, should success", func(t *testing.T) {
+ r := exampleRole("TestGetRole_getExistRole")
+ status, err := dao.CreateRole(context.TODO(), r)
+ assert.Nil(t, err)
+ assert.True(t, status.IsSucceed())
+ resp, status, err := dao.GetRole(context.TODO(), r.Name)
+ assert.Nil(t, err)
+ assert.True(t, status.IsSucceed())
+ assert.Equal(t, r.Name, resp.Name)
+ })
+}
+
+func TestEditRole(t *testing.T) {
+ t.Run("edit no exist role, should return: "+rbac.NewError(rbac.ErrRoleNotExist, "").Error(), func(t *testing.T) {
+ r := exampleRole("TestEditRole_editNoExistRole")
+ status, err := dao.EditRole(context.TODO(), r.Name, r)
+ assert.Nil(t, err)
+ assert.Equal(t, rbac.ErrRoleNotExist, status.GetCode())
+ })
+ t.Run("edit role, should success", func(t *testing.T) {
+ r := exampleRole("TestGetRole_editRole")
+ status, err := dao.CreateRole(context.TODO(), r)
+ assert.Nil(t, err)
+ assert.True(t, status.IsSucceed())
+
+ // edit
+ assert.Equal(t, 1, len(r.Perms))
+ r.Perms = []*rbac.Permission{
+ {
+ Resources: []*rbac.Resource{
+ {
+ Type: rbacsvc.ResourceService,
+ },
+ },
+ Verbs: []string{"*"},
+ },
+ {
+ Resources: []*rbac.Resource{
+ {
+ Type: rbacsvc.ResourceSchema,
+ },
+ },
+ Verbs: []string{"*"},
+ },
+ }
+ status, err = dao.EditRole(context.TODO(), r.Name, r)
+ assert.Nil(t, err)
+ assert.True(t, status.IsSucceed())
+
+ resp, status, err := dao.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) {
+ for _, name := range []string{rbac.RoleDeveloper, rbac.RoleDeveloper} {
+ status, err := dao.EditRole(context.TODO(), name, exampleRole(""))
+ assert.Nil(t, err)
+ assert.Equal(t, discovery.ErrForbidden, status.GetCode())
+ }
+ })
+}
+
+func TestDeleteRole(t *testing.T) {
+ t.Run("delete no exist role, should return: "+rbac.NewError(rbac.ErrRoleNotExist, "").Error(), func(t *testing.T) {
+ status, err := dao.DeleteRole(context.TODO(), "TestDeleteRole_deleteNoExistRole")
+ assert.Nil(t, err)
+ assert.Equal(t, rbac.ErrRoleNotExist, status.GetCode())
+ })
+ t.Run("delete role, should success", func(t *testing.T) {
+ r := exampleRole("TestDeleteRole_deleteRole")
+ status, err := dao.CreateRole(context.TODO(), r)
+ assert.Nil(t, err)
+ assert.True(t, status.IsSucceed())
+
+ status, err = dao.DeleteRole(context.TODO(), r.Name)
+ assert.Nil(t, err)
+ assert.True(t, status.IsSucceed())
+
+ exist, err := dao.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) {
+ for _, name := range []string{rbac.RoleDeveloper, rbac.RoleDeveloper} {
+ status, err := dao.DeleteRole(context.TODO(), name)
+ assert.Nil(t, err)
+ assert.Equal(t, discovery.ErrForbidden, status.GetCode())
+ }
+ })
+}
+
+func TestListRole(t *testing.T) {
+ t.Run("list role, should success", func(t *testing.T) {
+ roles, total, err := dao.ListRole(context.TODO())
+ assert.Nil(t, err)
+ assert.True(t, total > 0)
+ assert.Equal(t, int64(len(roles)), total)
+ })
+}
+
+func TestRoleExistt(t *testing.T) {
+ t.Run("check no exist role, should success and not exist", func(t *testing.T) {
+ exist, err := dao.RoleExist(context.TODO(), "TestRoleExist_checkNoExistRole")
+ assert.Nil(t, err)
+ assert.False(t, exist)
+ })
+}
diff --git a/server/service/rbac/decision.go b/server/service/rbac/decision.go
index 545f99b..52bf54c 100644
--- a/server/service/rbac/decision.go
+++ b/server/service/rbac/decision.go
@@ -83,16 +83,18 @@ func LabelMatched(targetResourceLabel map[string]string, permLabel map[string]st
func getPermsByRoles(ctx context.Context, roleList []string) ([]*rbac.Permission, error) {
var allPerms = make([]*rbac.Permission, 0)
- for i := 0; i < len(roleList); i++ {
- r, err := datasource.Instance().GetRole(ctx, roleList[i])
- if err != nil {
- return nil, err
+ for _, name := range roleList {
+ r, err := datasource.Instance().GetRole(ctx, name)
+ if err == nil {
+ allPerms = append(allPerms, r.Perms...)
+ continue
}
- if r == nil {
- log.Warnf("role [%s] has no any permissions", roleList[i])
+ if err == datasource.ErrRoleNotExist {
+ log.Warnf("role [%s] not exist", name)
continue
}
- allPerms = append(allPerms, r.Perms...)
+ log.Errorf(err, "get role [%s] failed", name)
+ return nil, err
}
return allPerms, nil
}
diff --git a/server/service/rbac/rbac.go b/server/service/rbac/rbac.go
index 76c7258..4807a34 100644
--- a/server/service/rbac/rbac.go
+++ b/server/service/rbac/rbac.go
@@ -21,12 +21,12 @@ import (
"context"
"crypto/rsa"
"errors"
+ "github.com/apache/servicecomb-service-center/datasource"
"github.com/apache/servicecomb-service-center/server/service/validator"
"io/ioutil"
"github.com/go-chassis/cari/rbac"
- "github.com/apache/servicecomb-service-center/datasource"
"github.com/apache/servicecomb-service-center/pkg/log"
"github.com/apache/servicecomb-service-center/server/config"
"github.com/apache/servicecomb-service-center/server/plugin/security/cipher"
@@ -70,8 +70,7 @@ func Init() {
}
readPrivateKey()
readPublicKey()
- initAdminRole()
- initDevRole()
+ initBuildInRole()
rbac.Add2WhiteAPIList(APITokenGranter)
log.Info("rbac is enabled")
}
diff --git a/server/service/rbac/rbac_test.go b/server/service/rbac/rbac_test.go
index 2d98516..65ae32f 100644
--- a/server/service/rbac/rbac_test.go
+++ b/server/service/rbac/rbac_test.go
@@ -21,11 +21,11 @@ import (
"context"
"github.com/apache/servicecomb-service-center/pkg/privacy"
"github.com/apache/servicecomb-service-center/server/config"
- "github.com/apache/servicecomb-service-center/server/service/rbac"
+ rbacsvc "github.com/apache/servicecomb-service-center/server/service/rbac"
"github.com/apache/servicecomb-service-center/server/service/rbac/dao"
_ "github.com/apache/servicecomb-service-center/test"
"github.com/astaxie/beego"
- rbacmodel "github.com/go-chassis/cari/rbac"
+ "github.com/go-chassis/cari/rbac"
"github.com/go-chassis/go-archaius"
"github.com/go-chassis/go-chassis/v2/security/authr"
"github.com/go-chassis/go-chassis/v2/security/secret"
@@ -61,12 +61,12 @@ func init() {
panic(err)
}
- archaius.Set(rbac.InitPassword, "Complicated_password1")
+ archaius.Set(rbacsvc.InitPassword, "Complicated_password1")
dao.DeleteAccount(context.Background(), "root")
dao.DeleteAccount(context.Background(), "a")
dao.DeleteAccount(context.Background(), "b")
- rbac.Init()
+ rbacsvc.Init()
}
func TestInitRBAC(t *testing.T) {
@@ -79,27 +79,27 @@ func TestInitRBAC(t *testing.T) {
assert.NoError(t, err)
claims, err := authr.Authenticate(context.Background(), token)
assert.NoError(t, err)
- assert.Equal(t, "root", claims.(map[string]interface{})[rbacmodel.ClaimsUser])
+ assert.Equal(t, "root", claims.(map[string]interface{})[rbac.ClaimsUser])
})
t.Run("second time init", func(t *testing.T) {
- rbac.Init()
+ rbacsvc.Init()
})
t.Run("change pwd,admin can change any one password", func(t *testing.T) {
- persisted := &rbacmodel.Account{Name: "a", Password: "Complicated_password1"}
+ persisted := &rbac.Account{Name: "a", Password: "Complicated_password1"}
err := dao.CreateAccount(context.Background(), persisted)
assert.NoError(t, err)
- err = rbac.ChangePassword(context.Background(), []string{rbacmodel.RoleAdmin}, "admin", &rbacmodel.Account{Name: "a", Password: "Complicated_password2"})
+ err = rbacsvc.ChangePassword(context.Background(), []string{rbac.RoleAdmin}, "admin", &rbac.Account{Name: "a", Password: "Complicated_password2"})
assert.NoError(t, err)
a, err := dao.GetAccount(context.Background(), "a")
assert.NoError(t, err)
assert.True(t, privacy.SamePassword(a.Password, "Complicated_password2"))
})
t.Run("change self password", func(t *testing.T) {
- err := dao.CreateAccount(context.Background(), &rbacmodel.Account{Name: "b", Password: "Complicated_password1"})
+ err := dao.CreateAccount(context.Background(), &rbac.Account{Name: "b", Password: "Complicated_password1"})
assert.NoError(t, err)
- err = rbac.ChangePassword(context.Background(), nil, "b", &rbacmodel.Account{Name: "b", CurrentPassword: "Complicated_password1", Password: "Complicated_password2"})
+ err = rbacsvc.ChangePassword(context.Background(), nil, "b", &rbac.Account{Name: "b", CurrentPassword: "Complicated_password1", Password: "Complicated_password2"})
assert.NoError(t, err)
a, err := dao.GetAccount(context.Background(), "b")
assert.NoError(t, err)
@@ -111,82 +111,6 @@ func TestInitRBAC(t *testing.T) {
assert.NoError(t, err)
assert.Greater(t, n, int64(2))
})
- dao.DeleteRole(context.Background(), "tester")
- t.Run("check is there exist role", func(t *testing.T) {
- exist, err := dao.RoleExist(context.Background(), "admin")
- assert.NoError(t, err)
- assert.Equal(t, true, exist)
-
- exist, err = dao.RoleExist(context.Background(), "developer")
- assert.NoError(t, err)
- assert.Equal(t, true, exist)
-
- exist, err = dao.RoleExist(context.Background(), "tester")
- assert.NoError(t, err)
- assert.Equal(t, false, exist)
- })
-
- t.Run("delete the default role", func(t *testing.T) {
- r, err := dao.DeleteRole(context.Background(), "admin")
- assert.NoError(t, err)
- assert.Equal(t, false, r)
-
- r, err = dao.DeleteRole(context.Background(), "developer")
- assert.NoError(t, err)
- assert.Equal(t, false, r)
- })
-
- t.Run("delete the not exist role", func(t *testing.T) {
- _, err := dao.DeleteRole(context.Background(), "tester")
- assert.NoError(t, err)
- })
-
- t.Run("list exist role", func(t *testing.T) {
- _, _, err := dao.ListRole(context.TODO())
- assert.NoError(t, err)
- })
-
- tester := &rbacmodel.Role{
- Name: "tester",
- Perms: []*rbacmodel.Permission{
- {
- Resources: []*rbacmodel.Resource{{Type: "service"}, {Type: "instance"}},
- Verbs: []string{"get", "create", "update"},
- },
- {
- Resources: []*rbacmodel.Resource{{Type: "rule"}},
- Verbs: []string{"*"},
- },
- },
- }
-
- t.Run("create new role success", func(t *testing.T) {
- err := dao.CreateRole(context.Background(), tester)
- assert.NoError(t, err)
-
- exist, err := dao.RoleExist(context.Background(), "tester")
- assert.NoError(t, err)
- assert.Equal(t, true, exist)
-
- r, err := dao.GetRole(context.Background(), "tester")
- assert.NoError(t, err)
- assert.NotNil(t, r)
- })
-
- t.Run("edit new role success", func(t *testing.T) {
- err := dao.EditRole(context.Background(), tester.Name, tester)
- assert.NoError(t, err)
-
- r, err := dao.GetRole(context.Background(), "tester")
- assert.NoError(t, err)
- assert.NotNil(t, r)
- })
-
- t.Run("delete the new role", func(t *testing.T) {
- r, err := dao.DeleteRole(context.Background(), "tester")
- assert.NoError(t, err)
- assert.Equal(t, true, r)
- })
}
func BenchmarkAuthResource_Login(b *testing.B) {
b.RunParallel(func(pb *testing.PB) {
diff --git a/server/service/rbac/role.go b/server/service/rbac/role.go
index dcba30d..1523579 100644
--- a/server/service/rbac/role.go
+++ b/server/service/rbac/role.go
@@ -19,7 +19,6 @@ package rbac
import (
"context"
-
"github.com/go-chassis/cari/rbac"
"github.com/apache/servicecomb-service-center/pkg/log"
@@ -28,26 +27,37 @@ import (
var roleMap = map[string]*rbac.Role{}
-// Assign resources to admin role, admin role own all permissions
-func initAdminRole() {
- roleMap["admin"] = &rbac.Role{
- Name: "admin",
+func init() {
+ // Assign resources to admin role, admin role own all permissions
+ roleMap[rbac.RoleAdmin] = &rbac.Role{
+ Name: rbac.RoleAdmin,
Perms: AdminPerms(),
}
- err := dao.CreateRole(context.Background(), roleMap["admin"])
- if err != nil {
- log.Errorf(err, "create admin role failed")
+ roleMap[rbac.RoleDeveloper] = &rbac.Role{
+ Name: rbac.RoleDeveloper,
+ Perms: DevPerms(),
}
}
-// Assign resources to developer role
-func initDevRole() {
- roleMap["developer"] = &rbac.Role{
- Name: "developer",
- Perms: DevPerms(),
+func initBuildInRole() {
+ for _, r := range roleMap {
+ createBuildInRole(r)
}
- err := dao.CreateRole(context.Background(), roleMap["developer"])
+}
+
+func createBuildInRole(r *rbac.Role) {
+ status, err := dao.CreateRole(context.Background(), r)
if err != nil {
- log.Errorf(err, "create developer role failed")
+ log.Fatalf(err, "create role [%s] failed", r.Name)
+ return
+ }
+ if status.IsSucceed() {
+ log.Infof("create role [%s] success", r.Name)
+ return
+ }
+ if status.Code == rbac.ErrRoleConflict {
+ log.Infof("role [%s] already exists", r.Name)
+ return
}
+ log.Fatalf(nil, "create role [%s] failed: %s", r.Name, status.GetMessage())
}