This is an automated email from the ASF dual-hosted git repository.
littlecui 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 7f9ac68 role binding check (#1027)
7f9ac68 is described below
commit 7f9ac68d6a91b42b87f9673bb7ab692896956db3
Author: Shawn <[email protected]>
AuthorDate: Mon May 31 17:06:35 2021 +0800
role binding check (#1027)
---
datasource/account.go | 1 +
datasource/account_test.go | 4 +--
datasource/etcd/account.go | 48 ++++++++++++++++++++++++++++----
datasource/etcd/path/key_generator.go | 15 +++++++++-
datasource/etcd/role.go | 16 +++++++++++
datasource/mongo/client/mongo.go | 38 ++++++++++++++++++++++++-
datasource/mongo/role.go | 8 ++++--
examples/etcd_data_struct.yaml | 4 ++-
server/resource/v4/role_resource.go | 6 ++++
server/resource/v4/role_resource_test.go | 38 +++++++++++++++++++------
10 files changed, 157 insertions(+), 21 deletions(-)
diff --git a/datasource/account.go b/datasource/account.go
index 0c66730..89f5c63 100644
--- a/datasource/account.go
+++ b/datasource/account.go
@@ -31,6 +31,7 @@ var (
ErrDeleteAccountFailed = errors.New("failed to delete account")
ErrQueryAccountFailed = errors.New("failed to query account")
ErrAccountNotExist = errors.New("account not exist")
+ ErrRoleBindingExist = errors.New("role is bind to account")
)
// AccountManager contains the RBAC CRUD
diff --git a/datasource/account_test.go b/datasource/account_test.go
index b61845d..759ad0f 100644
--- a/datasource/account_test.go
+++ b/datasource/account_test.go
@@ -83,9 +83,9 @@ func TestAccount(t *testing.T) {
a2.Password = "new-password"
err = datasource.Instance().UpdateAccount(context.Background(),
a2.Name, &a2)
assert.NoError(t, err)
- _, n, err :=
datasource.Instance().ListAccount(context.Background())
+ accounts, _, err :=
datasource.Instance().ListAccount(context.Background())
assert.NoError(t, err)
- assert.Equal(t, int64(2), n)
+ assert.GreaterOrEqual(t, len(accounts), 2)
_, err =
datasource.Instance().DeleteAccount(context.Background(), []string{a1.Name})
assert.NoError(t, err)
_, err =
datasource.Instance().DeleteAccount(context.Background(), []string{a2.Name})
diff --git a/datasource/etcd/account.go b/datasource/etcd/account.go
index 99bb296..bcf37e4 100644
--- a/datasource/etcd/account.go
+++ b/datasource/etcd/account.go
@@ -19,6 +19,7 @@ import (
"context"
"encoding/json"
"fmt"
+ "github.com/go-chassis/foundation/stringutil"
"strconv"
"time"
@@ -44,7 +45,6 @@ func (ds *DataSource) CreateAccount(ctx context.Context, a
*rbac.Account) error
log.Errorf(err, "can not release account lock")
}
}()
- name := path.GenerateRBACAccountKey(a.Name)
exist, err := datasource.Instance().AccountExist(ctx, a.Name)
if err != nil {
log.Errorf(err, "can not save account info")
@@ -63,12 +63,12 @@ func (ds *DataSource) CreateAccount(ctx context.Context, a
*rbac.Account) error
a.ID = util.GenerateUUID()
a.CreateTime = strconv.FormatInt(time.Now().Unix(), 10)
a.UpdateTime = a.CreateTime
- value, err := json.Marshal(a)
+ opts, err := GenAccountOpts(a, client.ActionPut)
if err != nil {
- log.Errorf(err, "account info is invalid")
+ log.Error("", err)
return err
}
- err = client.PutBytes(ctx, name, value)
+ err = client.BatchCommit(ctx, opts)
if err != nil {
log.Errorf(err, "can not save account info")
return err
@@ -76,7 +76,28 @@ func (ds *DataSource) CreateAccount(ctx context.Context, a
*rbac.Account) error
log.Info("create new account: " + a.ID)
return nil
}
+func GenAccountOpts(a *rbac.Account, action client.ActionType)
([]client.PluginOp, error) {
+ opts := make([]client.PluginOp, 0)
+ value, err := json.Marshal(a)
+ if err != nil {
+ log.Errorf(err, "account info is invalid")
+ return nil, err
+ }
+ opts = append(opts, client.PluginOp{
+ Key: stringutil.Str2bytes(path.GenerateAccountKey(a.Name)),
+ Value: value,
+ Action: action,
+ })
+ for _, r := range a.Roles {
+ opt := client.PluginOp{
+ Key:
stringutil.Str2bytes(path.GenRoleAccountIdxKey(r, a.Name)),
+ Action: action,
+ }
+ opts = append(opts, opt)
+ }
+ return opts, nil
+}
func (ds *DataSource) AccountExist(ctx context.Context, name string) (bool,
error) {
resp, err := client.Instance().Do(ctx, client.GET,
client.WithStrKey(path.GenerateRBACAccountKey(name)))
@@ -142,8 +163,23 @@ func (ds *DataSource) DeleteAccount(ctx context.Context,
names []string) (bool,
return false, nil
}
for _, name := range names {
- _, err := client.Instance().Do(ctx, client.DEL,
- client.WithStrKey(path.GenerateRBACAccountKey(name)))
+ a, err := ds.GetAccount(ctx, name)
+ if err != nil {
+ log.Error("", err)
+ continue //do not fail if some account is invalid
+
+ }
+ if a == nil {
+ log.Warn("can not find account")
+ continue
+ }
+ opts, err := GenAccountOpts(a, client.ActionDelete)
+ if err != nil {
+ log.Error("", err)
+ continue //do not fail if some account is invalid
+
+ }
+ err = client.BatchCommit(ctx, opts)
if err != nil {
log.Error(datasource.ErrDeleteAccountFailed.Error(),
err)
return false, err
diff --git a/datasource/etcd/path/key_generator.go
b/datasource/etcd/path/key_generator.go
index 8d6ce8b..f010a1a 100644
--- a/datasource/etcd/path/key_generator.go
+++ b/datasource/etcd/path/key_generator.go
@@ -76,7 +76,20 @@ func GenerateRBACRoleKey(name string) string {
name,
}, SPLIT)
}
-
+func GenRoleAccountIdxKey(role, account string) string {
+ return util.StringJoin([]string{
+ GetRootKey(),
+ "idx-role-account",
+ role, account,
+ }, SPLIT)
+}
+func GenRoleAccountPrefixIdxKey(role string) string {
+ return util.StringJoin([]string{
+ GetRootKey(),
+ "idx-role-account",
+ role,
+ }, SPLIT)
+}
func GenerateETCDProjectKey(domain, project string) string {
return util.StringJoin([]string{
GetProjectRootKey(domain),
diff --git a/datasource/etcd/role.go b/datasource/etcd/role.go
index 1cd803c..867f035 100644
--- a/datasource/etcd/role.go
+++ b/datasource/etcd/role.go
@@ -122,6 +122,14 @@ func (ds *DataSource) ListRole(ctx context.Context)
([]*rbac.Role, int64, error)
return roles, resp.Count, nil
}
func (ds *DataSource) DeleteRole(ctx context.Context, name string) (bool,
error) {
+ exists, err := RoleBindingExists(ctx, name)
+ if err != nil {
+ log.Error("", err)
+ return false, err
+ }
+ if exists {
+ return false, datasource.ErrRoleBindingExist
+ }
resp, err := client.Instance().Do(ctx, client.DEL,
client.WithStrKey(path.GenerateRBACRoleKey(name)))
if err != nil {
@@ -129,6 +137,14 @@ func (ds *DataSource) DeleteRole(ctx context.Context, name
string) (bool, error)
}
return resp.Succeeded, nil
}
+func RoleBindingExists(ctx context.Context, role string) (bool, error) {
+ _, total, err := client.List(ctx, path.GenRoleAccountPrefixIdxKey(role))
+ if err != nil {
+ log.Error("", err)
+ return false, err
+ }
+ return total > 0, nil
+}
func (ds *DataSource) UpdateRole(ctx context.Context, name string, role
*rbac.Role) error {
role.UpdateTime = strconv.FormatInt(time.Now().Unix(), 10)
value, err := json.Marshal(role)
diff --git a/datasource/mongo/client/mongo.go b/datasource/mongo/client/mongo.go
index 4f7e761..55dec95 100644
--- a/datasource/mongo/client/mongo.go
+++ b/datasource/mongo/client/mongo.go
@@ -61,7 +61,15 @@ type MongoOperation struct {
func GetMongoClient() *MongoClient {
return mc
}
-
+func DeleteDoc(ctx context.Context, Table string, filter interface{}, opts
...*options.DeleteOptions) (*mongo.DeleteResult, error) {
+ return mc.Delete(ctx, Table, filter, opts...)
+}
+func Find(ctx context.Context, Table string, filter interface{}, opts
...*options.FindOptions) (*mongo.Cursor, error) {
+ return mc.Find(ctx, Table, filter, opts...)
+}
+func Count(ctx context.Context, Table string, filter interface{}, opts
...*options.CountOptions) (int64, error) {
+ return mc.db.Collection(Table).CountDocuments(ctx, filter, opts...)
+}
func NewMongoClient(config storage.Options) {
inst := &MongoClient{}
if err := inst.Initialize(config); err != nil {
@@ -85,6 +93,34 @@ func (mc *MongoClient) Initialize(config storage.Options)
(err error) {
return nil
}
+// ExecTxn execute a transaction command
+// want to abort transaction, return error in cmd fn impl, otherwise it will
commit transaction
+func (mc *MongoClient) ExecTxn(ctx context.Context, cmd func(sessionContext
mongo.SessionContext) error) error {
+ session, err := mc.client.StartSession()
+ if err != nil {
+ return err
+ }
+ if err = session.StartTransaction(); err != nil {
+ return err
+ }
+ defer session.EndSession(ctx)
+ if err = mongo.WithSession(ctx, session, func(sc mongo.SessionContext)
error {
+ if err = cmd(sc); err != nil {
+ if err = session.AbortTransaction(sc); err != nil {
+ return err
+ }
+ return nil
+ } else {
+ if err = session.CommitTransaction(sc); err != nil {
+ return err
+ }
+ return nil
+ }
+ }); err != nil {
+ return err
+ }
+ return nil
+}
func (mc *MongoClient) Err() <-chan error {
return mc.err
}
diff --git a/datasource/mongo/role.go b/datasource/mongo/role.go
index 7ab4ce6..c4ab393 100644
--- a/datasource/mongo/role.go
+++ b/datasource/mongo/role.go
@@ -19,8 +19,8 @@ package mongo
import (
"context"
-
"github.com/go-chassis/cari/rbac"
+ "go.mongodb.org/mongo-driver/bson"
"github.com/apache/servicecomb-service-center/datasource"
"github.com/apache/servicecomb-service-center/datasource/mongo/client"
@@ -102,8 +102,12 @@ func (ds *DataSource) ListRole(ctx context.Context)
([]*rbac.Role, int64, error)
}
func (ds *DataSource) DeleteRole(ctx context.Context, name string) (bool,
error) {
+ n, err := client.Count(ctx, model.CollectionAccount, bson.M{"roles":
bson.M{"$in": []string{name}}})
+ if n > 0 {
+ return false, datasource.ErrRoleBindingExist
+ }
filter := mutil.NewFilter(mutil.RoleName(name))
- result, err := client.GetMongoClient().Delete(ctx,
model.CollectionRole, filter)
+ result, err := client.DeleteDoc(ctx, model.CollectionRole, filter)
if err != nil {
return false, err
}
diff --git a/examples/etcd_data_struct.yaml b/examples/etcd_data_struct.yaml
index 13c98d7..bb18424 100644
--- a/examples/etcd_data_struct.yaml
+++ b/examples/etcd_data_struct.yaml
@@ -130,7 +130,9 @@
"currentPassword": "password",
"status": "normal"
}
-
+# record role binding to account
+/cse-sr/idx-role-account/{role}/{account}:
+ {no value}
# domain
# /cse-sr/domains/{domain}
/cse-sr/domains/default:
diff --git a/server/resource/v4/role_resource.go
b/server/resource/v4/role_resource.go
index b768207..17e363b 100644
--- a/server/resource/v4/role_resource.go
+++ b/server/resource/v4/role_resource.go
@@ -19,6 +19,8 @@ package v4
import (
"encoding/json"
+ "errors"
+ "github.com/apache/servicecomb-service-center/datasource"
"io/ioutil"
"net/http"
@@ -138,6 +140,10 @@ func (rr *RoleResource) DeleteRole(w http.ResponseWriter,
req *http.Request) {
n := req.URL.Query().Get(":roleName")
status, err := dao.DeleteRole(req.Context(), n)
+ if errors.Is(err, datasource.ErrRoleBindingExist) {
+ rest.WriteError(w, discovery.ErrInvalidParams, errorsEx.MsgJSON)
+ return
+ }
if err != nil {
log.Error(errorsEx.MsgJSON, err)
rest.WriteError(w, discovery.ErrInternal, errorsEx.MsgJSON)
diff --git a/server/resource/v4/role_resource_test.go
b/server/resource/v4/role_resource_test.go
index 76ea993..4598496 100644
--- a/server/resource/v4/role_resource_test.go
+++ b/server/resource/v4/role_resource_test.go
@@ -18,11 +18,12 @@ import (
)
func TestRoleResource_CreateOrUpdateRole(t *testing.T) {
- var to = &rbacmodel.Token{}
+ var superToken = &rbacmodel.Token{}
ctx := context.TODO()
dao.DeleteAccount(ctx, "dev_test")
+ dao.DeleteAccount(ctx, "dev_test2")
dao.DeleteRole(ctx, "tester")
- t.Run("root login", func(t *testing.T) {
+ t.Run("root login,to get super token", func(t *testing.T) {
b, _ := json.Marshal(&rbacmodel.Account{Name: "root", Password:
"Complicated_password1"})
r, err := http.NewRequest(http.MethodPost, "/v4/token",
bytes.NewBuffer(b))
@@ -30,14 +31,14 @@ func TestRoleResource_CreateOrUpdateRole(t *testing.T) {
w := httptest.NewRecorder()
rest.GetRouter().ServeHTTP(w, r)
assert.Equal(t, http.StatusOK, w.Code)
- json.Unmarshal(w.Body.Bytes(), to)
+ json.Unmarshal(w.Body.Bytes(), superToken)
})
t.Run("create account dev_test and add a role", func(t *testing.T) {
b, _ := json.Marshal(&rbacmodel.Account{Name: "dev_test",
Password: "Complicated_password3", Roles: []string{"tester"}})
r, _ := http.NewRequest(http.MethodPost, "/v4/accounts",
bytes.NewBuffer(b))
- r.Header.Set(restful.HeaderAuth, "Bearer "+to.TokenStr)
+ r.Header.Set(restful.HeaderAuth, "Bearer "+superToken.TokenStr)
w := httptest.NewRecorder()
rest.GetRouter().ServeHTTP(w, r)
assert.Equal(t, http.StatusOK, w.Code)
@@ -64,13 +65,13 @@ func TestRoleResource_CreateOrUpdateRole(t *testing.T) {
})
r2, _ := http.NewRequest(http.MethodPost, "/v4/roles",
bytes.NewReader(b2))
- r2.Header.Set(restful.HeaderAuth, "Bearer "+to.TokenStr)
+ r2.Header.Set(restful.HeaderAuth, "Bearer "+superToken.TokenStr)
w2 := httptest.NewRecorder()
rest.GetRouter().ServeHTTP(w2, r2)
assert.Equal(t, http.StatusOK, w2.Code)
r3, _ := http.NewRequest(http.MethodGet, "/v4/roles", nil)
- r3.Header.Set(restful.HeaderAuth, "Bearer "+to.TokenStr)
+ r3.Header.Set(restful.HeaderAuth, "Bearer "+superToken.TokenStr)
w3 := httptest.NewRecorder()
rest.GetRouter().ServeHTTP(w3, r3)
assert.Equal(t, http.StatusOK, w3.Code)
@@ -85,13 +86,13 @@ func TestRoleResource_CreateOrUpdateRole(t *testing.T) {
},
})
r4, _ := http.NewRequest(http.MethodPut, "/v4/roles/tester",
bytes.NewReader(b4))
- r4.Header.Set(restful.HeaderAuth, "Bearer "+to.TokenStr)
+ r4.Header.Set(restful.HeaderAuth, "Bearer "+superToken.TokenStr)
w4 := httptest.NewRecorder()
rest.GetRouter().ServeHTTP(w4, r4)
assert.Equal(t, http.StatusOK, w4.Code)
})
- t.Run("Inquire role", func(t *testing.T) {
+ t.Run("get role", func(t *testing.T) {
b, _ := json.Marshal(&rbacmodel.Account{Name: "root", Password:
"Complicated_password1"})
r, _ := http.NewRequest(http.MethodPost, "/v4/token",
bytes.NewBuffer(b))
@@ -124,6 +125,27 @@ func TestRoleResource_CreateOrUpdateRole(t *testing.T) {
rest.GetRouter().ServeHTTP(w3, r3)
assert.Equal(t, http.StatusForbidden, w3.Code)
})
+
+ t.Run("delete tester role, it is bind to account, should fail", func(t
*testing.T) {
+ r3, _ := http.NewRequest(http.MethodDelete, "/v4/roles/tester",
nil)
+ r3.Header.Set(restful.HeaderAuth, "Bearer "+superToken.TokenStr)
+ w3 := httptest.NewRecorder()
+ rest.GetRouter().ServeHTTP(w3, r3)
+ assert.Equal(t, http.StatusBadRequest, w3.Code)
+ })
+ t.Run("delete account, then delete role should success", func(t
*testing.T) {
+ r3, _ := http.NewRequest(http.MethodDelete,
"/v4/accounts/dev_test", nil)
+ r3.Header.Set(restful.HeaderAuth, "Bearer "+superToken.TokenStr)
+ w3 := httptest.NewRecorder()
+ rest.GetRouter().ServeHTTP(w3, r3)
+ assert.Equal(t, http.StatusOK, w3.Code)
+
+ r4, _ := http.NewRequest(http.MethodDelete, "/v4/roles/tester",
nil)
+ r4.Header.Set(restful.HeaderAuth, "Bearer "+superToken.TokenStr)
+ w4 := httptest.NewRecorder()
+ rest.GetRouter().ServeHTTP(w4, r4)
+ assert.Equal(t, http.StatusOK, w4.Code)
+ })
}
func TestRoleResource_MoreRoles(t *testing.T) {
var to = &rbacmodel.Token{}
