diff --git a/internal/metastore/catalog.go b/internal/metastore/catalog.go index 9bc6854bf3..2e651d53e2 100644 --- a/internal/metastore/catalog.go +++ b/internal/metastore/catalog.go @@ -37,6 +37,7 @@ type RootCoordCatalog interface { ListRole(ctx context.Context, tenant string, entity *milvuspb.RoleEntity, includeUserInfo bool) ([]*milvuspb.RoleResult, error) ListUser(ctx context.Context, tenant string, entity *milvuspb.UserEntity, includeRoleInfo bool) ([]*milvuspb.UserResult, error) AlterGrant(ctx context.Context, tenant string, entity *milvuspb.GrantEntity, operateType milvuspb.OperatePrivilegeType) error + DeleteGrant(ctx context.Context, tenant string, role *milvuspb.RoleEntity) error ListGrant(ctx context.Context, tenant string, entity *milvuspb.GrantEntity) ([]*milvuspb.GrantEntity, error) ListPolicy(ctx context.Context, tenant string) ([]string, error) ListUserRole(ctx context.Context, tenant string) ([]string, error) diff --git a/internal/metastore/db/dao/collection_test.go b/internal/metastore/db/dao/collection_test.go index 4a3ea8f4ac..314a20cfa5 100644 --- a/internal/metastore/db/dao/collection_test.go +++ b/internal/metastore/db/dao/collection_test.go @@ -48,6 +48,7 @@ var ( roleTestDb dbmodel.IRoleDb userRoleTestDb dbmodel.IUserRoleDb grantTestDb dbmodel.IGrantDb + grantIDTestDb dbmodel.IGrantIDDb ) // TestMain is the first function executed in current package, we will do some initial here @@ -86,6 +87,7 @@ func TestMain(m *testing.M) { roleTestDb = NewMetaDomain().RoleDb(ctx) userRoleTestDb = NewMetaDomain().UserRoleDb(ctx) grantTestDb = NewMetaDomain().GrantDb(ctx) + grantIDTestDb = NewMetaDomain().GrantIDDb(ctx) // m.Run entry for executing tests os.Exit(m.Run()) diff --git a/internal/metastore/db/dao/common.go b/internal/metastore/db/dao/common.go index 0970aa896d..06724e14bc 100644 --- a/internal/metastore/db/dao/common.go +++ b/internal/metastore/db/dao/common.go @@ -56,3 +56,7 @@ func (d *metaDomain) UserRoleDb(ctx context.Context) dbmodel.IUserRoleDb { func (d *metaDomain) GrantDb(ctx context.Context) dbmodel.IGrantDb { return &grantDb{dbcore.GetDB(ctx)} } + +func (d *metaDomain) GrantIDDb(ctx context.Context) dbmodel.IGrantIDDb { + return &grantIDDb{dbcore.GetDB(ctx)} +} diff --git a/internal/metastore/db/dao/gran_id.go b/internal/metastore/db/dao/gran_id.go new file mode 100644 index 0000000000..c3509174dc --- /dev/null +++ b/internal/metastore/db/dao/gran_id.go @@ -0,0 +1,54 @@ +package dao + +import ( + "github.com/milvus-io/milvus/internal/log" + "github.com/milvus-io/milvus/internal/metastore/db/dbmodel" + "go.uber.org/zap" + "gorm.io/gorm" +) + +type grantIDDb struct { + db *gorm.DB +} + +func (g *grantIDDb) GetGrantIDs(tenantID string, grantID int64, privilege string, preloadGrant bool, preloadGrantor bool) ([]*dbmodel.GrantID, error) { + var ( + grantIDs []*dbmodel.GrantID + db *gorm.DB + err error + ) + db = g.db.Model(&dbmodel.GrantID{}). + Where(&dbmodel.GrantID{GrantID: grantID, Privilege: privilege}). + Where(dbmodel.GetCommonCondition(tenantID, false)) + if preloadGrant { + db = db.Preload("Grant") + } + if preloadGrantor { + db = db.Preload("Grantor") + } + err = db.Find(&grantIDs).Error + if err != nil { + log.Error("fail to get grant ids", zap.String("tenant_id", tenantID), zap.Int64("grantID", grantID), zap.String("privilege", privilege), zap.Error(err)) + return nil, err + } + return grantIDs, err +} + +func (g *grantIDDb) Insert(in *dbmodel.GrantID) error { + err := g.db.Create(in).Error + if err != nil { + log.Error("fail to insert the grant-id", zap.Any("in", in), zap.Error(err)) + } + return err +} + +func (g *grantIDDb) Delete(tenantID string, grantID int64, privilege string) error { + err := g.db.Model(dbmodel.GrantID{}). + Where(&dbmodel.GrantID{GrantID: grantID, Privilege: privilege}). + Where(dbmodel.GetCommonCondition(tenantID, false)). + Update("is_deleted", true).Error + if err != nil { + log.Error("fail to delete the user-role", zap.String("tenant_id", tenantID), zap.Int64("grantID", grantID), zap.String("privilege", privilege), zap.Error(err)) + } + return err +} diff --git a/internal/metastore/db/dao/gran_id_test.go b/internal/metastore/db/dao/gran_id_test.go new file mode 100644 index 0000000000..de3117dce1 --- /dev/null +++ b/internal/metastore/db/dao/gran_id_test.go @@ -0,0 +1,235 @@ +package dao + +import ( + "errors" + "testing" + + "github.com/DATA-DOG/go-sqlmock" + "github.com/milvus-io/milvus/internal/metastore/db/dbmodel" + "github.com/stretchr/testify/assert" +) + +func TestGrantID_GetGrantIDs(t *testing.T) { + var ( + grantID1 int64 = 10 + grantID2 int64 = 20 + grantorID1 int64 = 1 + grantorID2 int64 = 2 + privilege1 = "PrivilegeLoad" + privilege2 = "PrivilegeInsert" + grantIDs []*dbmodel.GrantID + err error + ) + + mock.ExpectQuery("SELECT * FROM `grant_id` WHERE `is_deleted` = ? AND `tenant_id` = ?"). + WithArgs(false, tenantID). + WillReturnRows( + sqlmock.NewRows([]string{"tenant_id", "grant_id", "grantor_id", "privilege"}). + AddRow(tenantID, grantID1, grantorID1, privilege1). + AddRow(tenantID, grantID2, grantorID2, privilege2)) + + grantIDs, err = grantIDTestDb.GetGrantIDs(tenantID, 0, "", false, false) + mock.MatchExpectationsInOrder(false) + assert.NoError(t, err) + assert.Equal(t, 2, len(grantIDs)) + assert.Equal(t, grantID1, grantIDs[0].GrantID) + assert.Equal(t, grantorID2, grantIDs[1].GrantorID) + assert.Equal(t, privilege2, grantIDs[1].Privilege) + + mock.ExpectQuery("SELECT * FROM `grant_id` WHERE `is_deleted` = ? AND `tenant_id` = ?"). + WithArgs(false, tenantID). + WillReturnError(errors.New("test error")) + _, err = grantIDTestDb.GetGrantIDs(tenantID, 0, "", false, false) + mock.MatchExpectationsInOrder(false) + assert.Error(t, err) +} + +func TestGrantID_GetGrantIDs_Preload(t *testing.T) { + var ( + grantID1 int64 = 10 + grantID2 int64 = 20 + grantorID1 int64 = 1 + grantorID2 int64 = 2 + privilege1 = "PrivilegeLoad" + privilege2 = "PrivilegeInsert" + grantIDs []*dbmodel.GrantID + err error + ) + + mock.ExpectQuery("SELECT * FROM `grant_id` WHERE `is_deleted` = ? AND `tenant_id` = ?"). + WithArgs(false, tenantID). + WillReturnRows( + sqlmock.NewRows([]string{"tenant_id", "grant_id", "grantor_id", "privilege"}). + AddRow(tenantID, grantID1, grantorID1, privilege1). + AddRow(tenantID, grantID2, grantorID2, privilege2)) + + mock.ExpectQuery("SELECT * FROM `grant` WHERE `grant`.`id` IN (?,?)"). + WithArgs(grantID1, grantID2). + WillReturnRows( + sqlmock.NewRows([]string{"id", "tenant_id", "object"}). + AddRow(grantID1, tenantID, "obj1"). + AddRow(grantID2, tenantID, "obj2")) + + mock.ExpectQuery("SELECT * FROM `credential_users` WHERE `credential_users`.`id` IN (?,?)"). + WithArgs(grantorID1, grantorID2). + WillReturnRows( + sqlmock.NewRows([]string{"id", "tenant_id", "username"}). + AddRow(grantorID1, tenantID, "fo1"). + AddRow(grantorID2, tenantID, "fo2")) + + grantIDs, err = grantIDTestDb.GetGrantIDs(tenantID, 0, "", true, true) + mock.MatchExpectationsInOrder(false) + assert.NoError(t, err) + assert.Equal(t, 2, len(grantIDs)) + assert.Equal(t, grantID1, grantIDs[0].GrantID) + assert.Equal(t, "obj1", grantIDs[0].Grant.Object) + assert.Equal(t, grantorID2, grantIDs[1].GrantorID) + assert.Equal(t, privilege2, grantIDs[1].Privilege) + assert.Equal(t, "fo2", grantIDs[1].Grantor.Username) + + mock.ExpectQuery("SELECT * FROM `grant_id` WHERE `is_deleted` = ? AND `tenant_id` = ?"). + WithArgs(false, tenantID). + WillReturnError(errors.New("test error")) + _, err = grantIDTestDb.GetGrantIDs(tenantID, 0, "", true, true) + mock.MatchExpectationsInOrder(false) + assert.Error(t, err) +} + +func TestGrantID_GetGrantIDs_WithGrant(t *testing.T) { + var ( + grantID1 int64 = 10 + grantorID1 int64 = 1 + grantorID2 int64 = 2 + privilege1 = "PrivilegeLoad" + privilege2 = "PrivilegeInsert" + grantIDs []*dbmodel.GrantID + err error + ) + + mock.ExpectQuery("SELECT * FROM `grant_id` WHERE `grant_id`.`grant_id` = ? AND `is_deleted` = ? AND `tenant_id` = ?"). + WithArgs(grantID1, false, tenantID). + WillReturnRows( + sqlmock.NewRows([]string{"tenant_id", "grant_id", "grantor_id", "privilege"}). + AddRow(tenantID, grantID1, grantorID1, privilege1). + AddRow(tenantID, grantID1, grantorID2, privilege2)) + + grantIDs, err = grantIDTestDb.GetGrantIDs(tenantID, grantID1, "", false, false) + mock.MatchExpectationsInOrder(false) + assert.NoError(t, err) + assert.Equal(t, 2, len(grantIDs)) + assert.Equal(t, grantID1, grantIDs[0].GrantID) + assert.Equal(t, grantorID2, grantIDs[1].GrantorID) + assert.Equal(t, privilege2, grantIDs[1].Privilege) +} + +func TestGrantID_GetGrantIDs_WithGrantAndPrivilege(t *testing.T) { + var ( + grantID1 int64 = 10 + grantorID1 int64 = 1 + privilege1 = "PrivilegeLoad" + grantIDs []*dbmodel.GrantID + err error + ) + + mock.ExpectQuery("SELECT * FROM `grant_id` WHERE `grant_id`.`grant_id` = ? AND `grant_id`.`privilege` = ? AND `is_deleted` = ? AND `tenant_id` = ?"). + WithArgs(grantID1, privilege1, false, tenantID). + WillReturnRows( + sqlmock.NewRows([]string{"tenant_id", "grant_id", "grantor_id", "privilege"}). + AddRow(tenantID, grantID1, grantorID1, privilege1)) + + grantIDs, err = grantIDTestDb.GetGrantIDs(tenantID, grantID1, privilege1, false, false) + mock.MatchExpectationsInOrder(false) + assert.NoError(t, err) + assert.Equal(t, 1, len(grantIDs)) + assert.Equal(t, grantID1, grantIDs[0].GrantID) + assert.Equal(t, privilege1, grantIDs[0].Privilege) +} + +func TestGrantID_Insert(t *testing.T) { + var ( + grantID *dbmodel.GrantID + err error + ) + grantID = &dbmodel.GrantID{ + Base: GetBase(), + GrantID: 1, + GrantorID: 10, + Privilege: "PrivilegeLoad", + } + + mock.ExpectBegin() + mock.ExpectExec("INSERT INTO `grant_id` (`tenant_id`,`is_deleted`,`created_at`,`updated_at`,`grant_id`,`privilege`,`grantor_id`) VALUES (?,?,?,?,?,?,?)"). + WithArgs(grantID.TenantID, grantID.IsDeleted, grantID.CreatedAt, grantID.UpdatedAt, grantID.GrantID, grantID.Privilege, grantID.GrantorID). + WillReturnResult(sqlmock.NewResult(1, 1)) + mock.ExpectCommit() + err = grantIDTestDb.Insert(grantID) + mock.MatchExpectationsInOrder(false) + assert.NoError(t, err) + +} + +func TestGrantID_Insert_Error(t *testing.T) { + var ( + grantID *dbmodel.GrantID + err error + ) + grantID = &dbmodel.GrantID{ + Base: GetBase(), + GrantID: 1, + GrantorID: 10, + Privilege: "PrivilegeLoad", + } + + mock.ExpectBegin() + mock.ExpectExec("INSERT INTO `grant_id` (`tenant_id`,`is_deleted`,`created_at`,`updated_at`,`grant_id`,`privilege`,`grantor_id`) VALUES (?,?,?,?,?,?,?)"). + WithArgs(grantID.TenantID, grantID.IsDeleted, grantID.CreatedAt, grantID.UpdatedAt, grantID.GrantID, grantID.Privilege, grantID.GrantorID). + WillReturnError(errors.New("test error")) + mock.ExpectRollback() + err = grantIDTestDb.Insert(grantID) + mock.MatchExpectationsInOrder(false) + assert.Error(t, err) +} + +func TestGrantID_Delete(t *testing.T) { + var ( + grantID *dbmodel.GrantID + err error + ) + grantID = &dbmodel.GrantID{ + Base: GetBase(), + GrantID: 1, + GrantorID: 10, + Privilege: "PrivilegeLoad", + } + + mock.ExpectBegin() + mock.ExpectExec("UPDATE `grant_id` SET `is_deleted`=?,`updated_at`=? WHERE `grant_id`.`grant_id` = ? AND `grant_id`.`privilege` = ? AND `is_deleted` = ? AND `tenant_id` = ?"). + WithArgs(true, AnyTime{}, grantID.GrantID, grantID.Privilege, grantID.IsDeleted, grantID.TenantID). + WillReturnResult(sqlmock.NewResult(1, 1)) + mock.ExpectCommit() + err = grantIDTestDb.Delete(grantID.TenantID, grantID.GrantID, grantID.Privilege) + mock.MatchExpectationsInOrder(false) + assert.NoError(t, err) +} + +func TestGrantID_Delete_Error(t *testing.T) { + var ( + grantID *dbmodel.GrantID + err error + ) + grantID = &dbmodel.GrantID{ + Base: GetBase(), + GrantID: 1, + GrantorID: 10, + Privilege: "PrivilegeLoad", + } + + mock.ExpectBegin() + mock.ExpectExec("UPDATE `grant_id` SET `is_deleted`=?,`updated_at`=? WHERE `grant_id`.`grant_id` = ? AND `grant_id`.`privilege` = ? AND `is_deleted` = ? AND `tenant_id` = ?"). + WithArgs(true, AnyTime{}, grantID.GrantID, grantID.Privilege, grantID.IsDeleted, grantID.TenantID). + WillReturnError(errors.New("test error")) + mock.ExpectCommit() + err = grantIDTestDb.Delete(grantID.TenantID, grantID.GrantID, grantID.Privilege) + mock.MatchExpectationsInOrder(false) + assert.Error(t, err) +} diff --git a/internal/metastore/db/dao/grant.go b/internal/metastore/db/dao/grant.go index 6b94614dae..1233f021c6 100644 --- a/internal/metastore/db/dao/grant.go +++ b/internal/metastore/db/dao/grant.go @@ -1,11 +1,6 @@ package dao import ( - "errors" - "fmt" - - "github.com/milvus-io/milvus/internal/common" - "github.com/milvus-io/milvus/internal/log" "github.com/milvus-io/milvus/internal/metastore/db/dbmodel" "go.uber.org/zap" @@ -21,7 +16,11 @@ func (g *grantDb) GetGrants(tenantID string, roleID int64, object string, object grants []*dbmodel.Grant err error ) - err = g.db.Model(&dbmodel.Grant{}).Where(&dbmodel.Grant{RoleID: roleID, Object: object, ObjectName: objectName}).Where(dbmodel.GetCommonCondition(tenantID, false)).Preload("Role").Find(&grants).Error + err = g.db.Model(&dbmodel.Grant{}). + Where(&dbmodel.Grant{RoleID: roleID, Object: object, ObjectName: objectName}). + Where(dbmodel.GetCommonCondition(tenantID, false)). + Preload("Role"). + Find(&grants).Error if err != nil { log.Error("fail to get grants", zap.String("tenant_id", tenantID), zap.Int64("roleID", roleID), zap.String("object", object), zap.String("object_name", objectName), zap.Error(err)) return nil, err @@ -30,64 +29,20 @@ func (g *grantDb) GetGrants(tenantID string, roleID int64, object string, object } func (g *grantDb) Insert(in *dbmodel.Grant) error { - var ( - sqlWhere = &dbmodel.Grant{RoleID: in.RoleID, Object: in.Object, ObjectName: in.ObjectName} - dbGrant *dbmodel.Grant - newDbDetail string - err error - ) - err = g.db.Model(&dbmodel.Grant{}).Where(sqlWhere).Where(dbmodel.GetCommonCondition(in.TenantID, false)).Take(&dbGrant).Error - if errors.Is(err, gorm.ErrRecordNotFound) { - err = g.db.Create(in).Error - if err != nil { - log.Error("fail to insert the grant", zap.Any("in", in), zap.Error(err)) - } - return err - } + err := g.db.Create(in).Error if err != nil { - log.Error("fail to take the origin grant", zap.Any("in", in), zap.Error(err)) - return err - } - if newDbDetail, err = dbmodel.EncodeGrantDetailForString(dbGrant.Detail, in.Detail, true); err != nil { - log.Error("fail to encode the grant detail", zap.Any("in", in), zap.Error(err)) - return err - } - err = g.db.Model(dbmodel.Grant{}).Where(sqlWhere).Where(dbmodel.GetCommonCondition(in.TenantID, false)).Update("detail", newDbDetail).Error - if err != nil { - log.Error("fail to update the grant", zap.Any("in", in), zap.Error(err)) + log.Error("fail to insert the grant", zap.Any("in", in), zap.Error(err)) } return err } -func (g *grantDb) Delete(tenantID string, roleID int64, object string, objectName string, privilege string) error { - var ( - sqlWhere = &dbmodel.Grant{RoleID: roleID, Object: object, ObjectName: objectName} - dbGrant *dbmodel.Grant - newDbDetail string - db *gorm.DB - err error - ) - - err = g.db.Model(&dbmodel.Grant{}).Where(sqlWhere).Where(dbmodel.GetCommonCondition(tenantID, false)).Take(&dbGrant).Error - if errors.Is(err, gorm.ErrRecordNotFound) { - return common.NewIgnorableError(fmt.Errorf("the privilege[%s-%s-%s] isn't granted", object, objectName, privilege)) - } +func (g *grantDb) Delete(tenantID string, roleID int64, object string, objectName string) error { + err := g.db.Model(dbmodel.Grant{}). + Where(&dbmodel.Grant{RoleID: roleID, Object: object, ObjectName: objectName}). + Where(dbmodel.GetCommonCondition(tenantID, false)). + Update("is_deleted", true).Error if err != nil { - log.Error("fail to take the origin grant", zap.Any("where", sqlWhere), zap.Error(err)) - return err - } - if newDbDetail, err = dbmodel.EncodeGrantDetail(dbGrant.Detail, "", privilege, false); err != nil { - log.Error("fail to encode the grant detail", zap.Any("detail", dbGrant.Detail), zap.String("privilege", privilege), zap.Error(err)) - return err - } - db = g.db.Model(dbmodel.Grant{}).Where(sqlWhere).Where(dbmodel.GetCommonCondition(tenantID, false)) - if newDbDetail == "" { - err = db.Update("is_deleted", true).Error - } else { - err = db.Update("detail", newDbDetail).Error - } - if err != nil { - log.Error("fail to delete the grant", zap.Bool("is_delete", newDbDetail == ""), zap.Any("where", sqlWhere), zap.String("privilege", privilege), zap.Error(err)) + log.Error("fail to delete the grant", zap.String("tenant_id", tenantID), zap.Int64("roleID", roleID), zap.String("object", object), zap.String("object_name", objectName), zap.Error(err)) } return err } diff --git a/internal/metastore/db/dao/grant_test.go b/internal/metastore/db/dao/grant_test.go index 595da98159..27becdb45d 100644 --- a/internal/metastore/db/dao/grant_test.go +++ b/internal/metastore/db/dao/grant_test.go @@ -4,10 +4,6 @@ import ( "errors" "testing" - "github.com/milvus-io/milvus/internal/common" - - "gorm.io/gorm" - "github.com/DATA-DOG/go-sqlmock" "github.com/milvus-io/milvus/internal/metastore/db/dbmodel" "github.com/stretchr/testify/assert" @@ -20,18 +16,15 @@ func TestGrant_GetGrants(t *testing.T) { object = "Collection" objectName = "col1" grants []*dbmodel.Grant - getQuery func() *sqlmock.ExpectedQuery err error ) - getQuery = func() *sqlmock.ExpectedQuery { - return mock.ExpectQuery("SELECT * FROM `grant` WHERE `is_deleted` = ? AND `tenant_id` = ?"). - WithArgs(false, tenantID) - } - getQuery().WillReturnRows( - sqlmock.NewRows([]string{"tenant_id", "role_id", "object", "object_name"}). - AddRow(tenantID, roleID1, object, objectName). - AddRow(tenantID, roleID2, object, objectName)) + mock.ExpectQuery("SELECT * FROM `grant` WHERE `is_deleted` = ? AND `tenant_id` = ?"). + WithArgs(false, tenantID). + WillReturnRows( + sqlmock.NewRows([]string{"tenant_id", "role_id", "object", "object_name"}). + AddRow(tenantID, roleID1, object, objectName). + AddRow(tenantID, roleID2, object, objectName)) mock.ExpectQuery("SELECT * FROM `role` WHERE `role`.`id` IN (?,?)"). WithArgs(roleID1, roleID2). @@ -41,16 +34,19 @@ func TestGrant_GetGrants(t *testing.T) { AddRow(roleID2, tenantID, "foo2")) grants, err = grantTestDb.GetGrants(tenantID, 0, "", "") + mock.MatchExpectationsInOrder(false) assert.NoError(t, err) assert.Equal(t, 2, len(grants)) assert.Equal(t, "foo2", grants[1].Role.Name) assert.Equal(t, object, grants[0].Object) assert.Equal(t, objectName, grants[0].ObjectName) - getQuery().WillReturnError(errors.New("test error")) + mock.ExpectQuery("SELECT * FROM `grant` WHERE `is_deleted` = ? AND `tenant_id` = ?"). + WithArgs(false, tenantID). + WillReturnError(errors.New("test error")) _, err = grantTestDb.GetGrants(tenantID, 0, "", "") + mock.MatchExpectationsInOrder(false) assert.Error(t, err) - } func TestGrant_GetGrantsWithRoleID(t *testing.T) { @@ -61,18 +57,15 @@ func TestGrant_GetGrantsWithRoleID(t *testing.T) { object2 = "Global" objectName2 = "*" grants []*dbmodel.Grant - getQuery func() *sqlmock.ExpectedQuery err error ) - getQuery = func() *sqlmock.ExpectedQuery { - return mock.ExpectQuery("SELECT * FROM `grant` WHERE `grant`.`role_id` = ? AND `is_deleted` = ? AND `tenant_id` = ?"). - WithArgs(roleID1, false, tenantID) - } - getQuery().WillReturnRows( - sqlmock.NewRows([]string{"tenant_id", "role_id", "object", "object_name"}). - AddRow(tenantID, roleID1, object1, objectName1). - AddRow(tenantID, roleID1, object2, objectName2)) + mock.ExpectQuery("SELECT * FROM `grant` WHERE `grant`.`role_id` = ? AND `is_deleted` = ? AND `tenant_id` = ?"). + WithArgs(roleID1, false, tenantID). + WillReturnRows( + sqlmock.NewRows([]string{"tenant_id", "role_id", "object", "object_name"}). + AddRow(tenantID, roleID1, object1, objectName1). + AddRow(tenantID, roleID1, object2, objectName2)) mock.ExpectQuery("SELECT * FROM `role` WHERE `role`.`id` = ?"). WithArgs(roleID1). @@ -81,6 +74,7 @@ func TestGrant_GetGrantsWithRoleID(t *testing.T) { AddRow(roleID1, tenantID, "foo1")) grants, err = grantTestDb.GetGrants(tenantID, int64(roleID1), "", "") + mock.MatchExpectationsInOrder(false) assert.NoError(t, err) assert.Equal(t, 2, len(grants)) assert.Equal(t, "foo1", grants[0].Role.Name) @@ -93,21 +87,15 @@ func TestGrant_GetGrantsWithObject(t *testing.T) { roleID = 10 object = "Collection" objectName = "col1" - detail1 = "privilege1..." - detail2 = "privilege2..." grants []*dbmodel.Grant - getQuery func() *sqlmock.ExpectedQuery err error ) - getQuery = func() *sqlmock.ExpectedQuery { - return mock.ExpectQuery("SELECT * FROM `grant` WHERE `grant`.`role_id` = ? AND `grant`.`object` = ? AND `grant`.`object_name` = ? AND `is_deleted` = ? AND `tenant_id` = ?"). - WithArgs(roleID, object, objectName, false, tenantID) - } - getQuery().WillReturnRows( - sqlmock.NewRows([]string{"tenant_id", "role_id", "object", "object_name", "detail"}). - AddRow(tenantID, roleID, object, objectName, detail1). - AddRow(tenantID, roleID, object, objectName, detail2)) + mock.ExpectQuery("SELECT * FROM `grant` WHERE `grant`.`role_id` = ? AND `grant`.`object` = ? AND `grant`.`object_name` = ? AND `is_deleted` = ? AND `tenant_id` = ?"). + WithArgs(roleID, object, objectName, false, tenantID). + WillReturnRows( + sqlmock.NewRows([]string{"tenant_id", "role_id", "object", "object_name"}). + AddRow(tenantID, roleID, object, objectName)) mock.ExpectQuery("SELECT * FROM `role` WHERE `role`.`id` = ?"). WithArgs(roleID). @@ -116,12 +104,12 @@ func TestGrant_GetGrantsWithObject(t *testing.T) { AddRow(roleID, tenantID, "foo1")) grants, err = grantTestDb.GetGrants(tenantID, int64(roleID), object, objectName) + mock.MatchExpectationsInOrder(false) assert.NoError(t, err) - assert.Equal(t, 2, len(grants)) + assert.Equal(t, 1, len(grants)) assert.Equal(t, "foo1", grants[0].Role.Name) - assert.Equal(t, object, grants[1].Object) - assert.Equal(t, objectName, grants[1].ObjectName) - assert.Equal(t, detail2, grants[1].Detail) + assert.Equal(t, object, grants[0].Object) + assert.Equal(t, objectName, grants[0].ObjectName) } func TestGrant_Insert(t *testing.T) { @@ -134,18 +122,15 @@ func TestGrant_Insert(t *testing.T) { RoleID: 1, Object: "Global", ObjectName: "Col", - Detail: "[[\"admin\",\"PrivilegeIndexDetail1\"]]", } - mock.ExpectQuery("SELECT * FROM `grant` WHERE `grant`.`role_id` = ? AND `grant`.`object` = ? AND `grant`.`object_name` = ? AND `is_deleted` = ? AND `tenant_id` = ? LIMIT 1"). - WithArgs(grant.RoleID, grant.Object, grant.ObjectName, grant.IsDeleted, grant.TenantID). - WillReturnError(gorm.ErrRecordNotFound) mock.ExpectBegin() - mock.ExpectExec("INSERT INTO `grant` (`tenant_id`,`is_deleted`,`created_at`,`updated_at`,`role_id`,`object`,`object_name`,`detail`) VALUES (?,?,?,?,?,?,?,?)"). - WithArgs(grant.TenantID, grant.IsDeleted, grant.CreatedAt, grant.UpdatedAt, grant.RoleID, grant.Object, grant.ObjectName, grant.Detail). + mock.ExpectExec("INSERT INTO `grant` (`tenant_id`,`is_deleted`,`created_at`,`updated_at`,`role_id`,`object`,`object_name`) VALUES (?,?,?,?,?,?,?)"). + WithArgs(grant.TenantID, grant.IsDeleted, grant.CreatedAt, grant.UpdatedAt, grant.RoleID, grant.Object, grant.ObjectName). WillReturnResult(sqlmock.NewResult(1, 1)) mock.ExpectCommit() err = grantTestDb.Insert(grant) + mock.MatchExpectationsInOrder(false) assert.NoError(t, err) } @@ -160,22 +145,19 @@ func TestGrant_Insert_Error(t *testing.T) { RoleID: 1, Object: "Global", ObjectName: "Col", - Detail: "[[\"admin\",\"PrivilegeIndexDetail2\"]]", } - mock.ExpectQuery("SELECT * FROM `grant` WHERE `grant`.`role_id` = ? AND `grant`.`object` = ? AND `grant`.`object_name` = ? AND `is_deleted` = ? AND `tenant_id` = ? LIMIT 1"). - WithArgs(grant.RoleID, grant.Object, grant.ObjectName, grant.IsDeleted, grant.TenantID). - WillReturnError(gorm.ErrRecordNotFound) mock.ExpectBegin() - mock.ExpectExec("INSERT INTO `grant` (`tenant_id`,`is_deleted`,`created_at`,`updated_at`,`role_id`,`object`,`object_name`,`detail`) VALUES (?,?,?,?,?,?,?,?)"). - WithArgs(grant.TenantID, grant.IsDeleted, grant.CreatedAt, grant.UpdatedAt, grant.RoleID, grant.Object, grant.ObjectName, grant.Detail). + mock.ExpectExec("INSERT INTO `grant` (`tenant_id`,`is_deleted`,`created_at`,`updated_at`,`role_id`,`object`,`object_name`) VALUES (?,?,?,?,?,?,?)"). + WithArgs(grant.TenantID, grant.IsDeleted, grant.CreatedAt, grant.UpdatedAt, grant.RoleID, grant.Object, grant.ObjectName). WillReturnError(errors.New("test error")) mock.ExpectRollback() err = grantTestDb.Insert(grant) + mock.MatchExpectationsInOrder(false) assert.Error(t, err) } -func TestGrant_Insert_SelectError(t *testing.T) { +func TestGrant_Delete(t *testing.T) { var ( grant *dbmodel.Grant err error @@ -185,235 +167,36 @@ func TestGrant_Insert_SelectError(t *testing.T) { RoleID: 1, Object: "Global", ObjectName: "Col", - Detail: "[[\"admin\",\"PrivilegeIndexDetail3\"]]", } - mock.ExpectQuery("SELECT * FROM `grant` WHERE `grant`.`role_id` = ? AND `grant`.`object` = ? AND `grant`.`object_name` = ? AND `is_deleted` = ? AND `tenant_id` = ? LIMIT 1"). - WithArgs(grant.RoleID, grant.Object, grant.ObjectName, grant.IsDeleted, grant.TenantID). - WillReturnError(errors.New("test error")) - err = grantTestDb.Insert(grant) - assert.Error(t, err) -} - -func TestGrant_InsertDecode(t *testing.T) { - var ( - grant *dbmodel.Grant - err error - ) - grant = &dbmodel.Grant{ - Base: GetBase(), - RoleID: 1, - Object: "Global", - ObjectName: "Col", - Detail: "[[\"admin\",\"PrivilegeIndexDetail\"]]", - } - - mock.ExpectQuery("SELECT * FROM `grant` WHERE `grant`.`role_id` = ? AND `grant`.`object` = ? AND `grant`.`object_name` = ? AND `is_deleted` = ? AND `tenant_id` = ? LIMIT 1"). - WithArgs(grant.RoleID, grant.Object, grant.ObjectName, grant.IsDeleted, grant.TenantID). - WillReturnRows( - sqlmock.NewRows([]string{"tenant_id", "role_id", "object", "object_name", "detail"}). - AddRow(tenantID, grant.RoleID, grant.Object, grant.ObjectName, "aaa")) - err = grantTestDb.Insert(grant) - assert.Error(t, err) -} - -func TestGrant_InsertUpdate(t *testing.T) { - var ( - originDetail = "[[\"admin\",\"PrivilegeLoad\"]]" - grant *dbmodel.Grant - expectDetail string - err error - ) - grant = &dbmodel.Grant{ - Base: GetBase(), - RoleID: 1, - Object: "Global", - ObjectName: "Col", - Detail: "[[\"admin\",\"PrivilegeIndexDetail\"]]", - } - - expectDetail, _ = dbmodel.EncodeGrantDetailForString(originDetail, grant.Detail, true) - mock.ExpectQuery("SELECT * FROM `grant` WHERE `grant`.`role_id` = ? AND `grant`.`object` = ? AND `grant`.`object_name` = ? AND `is_deleted` = ? AND `tenant_id` = ? LIMIT 1"). - WithArgs(grant.RoleID, grant.Object, grant.ObjectName, grant.IsDeleted, grant.TenantID). - WillReturnRows( - sqlmock.NewRows([]string{"tenant_id", "role_id", "object", "object_name", "detail"}). - AddRow(tenantID, grant.RoleID, grant.Object, grant.ObjectName, originDetail)) - mock.ExpectBegin() - mock.ExpectExec("UPDATE `grant` SET `detail`=?,`updated_at`=? WHERE `grant`.`role_id` = ? AND `grant`.`object` = ? AND `grant`.`object_name` = ? AND `is_deleted` = ? AND `tenant_id` = ?"). - WithArgs(expectDetail, AnyTime{}, grant.RoleID, grant.Object, grant.ObjectName, grant.IsDeleted, grant.TenantID). - WillReturnResult(sqlmock.NewResult(1, 1)) - mock.ExpectCommit() - err = grantTestDb.Insert(grant) - assert.NoError(t, err) -} - -func TestGrant_InsertUpdateError(t *testing.T) { - var ( - originDetail = "[[\"admin\",\"PrivilegeIndexDetail\"]]" - grant *dbmodel.Grant - err error - ) - grant = &dbmodel.Grant{ - Base: GetBase(), - RoleID: 1, - Object: "Global", - ObjectName: "Col", - Detail: "[[\"admin\",\"PrivilegeIndexDetail\"]]", - } - - mock.ExpectQuery("SELECT * FROM `grant` WHERE `grant`.`role_id` = ? AND `grant`.`object` = ? AND `grant`.`object_name` = ? AND `is_deleted` = ? AND `tenant_id` = ? LIMIT 1"). - WithArgs(grant.RoleID, grant.Object, grant.ObjectName, grant.IsDeleted, grant.TenantID). - WillReturnRows( - sqlmock.NewRows([]string{"tenant_id", "role_id", "object", "object_name", "detail"}). - AddRow(tenantID, grant.RoleID, grant.Object, grant.ObjectName, originDetail)) - err = grantTestDb.Insert(grant) - assert.Error(t, err) - assert.True(t, common.IsIgnorableError(err)) -} - -func TestGrant_DeleteWithoutPrivilege(t *testing.T) { - var ( - grant *dbmodel.Grant - privilege = "PrivilegeIndexDetail" - err error - ) - grant = &dbmodel.Grant{ - Base: GetBase(), - RoleID: 1, - Object: "Global", - ObjectName: "Col", - Detail: "[[\"admin\",\"PrivilegeIndexDetail\"]]", - } - - mock.ExpectQuery("SELECT * FROM `grant` WHERE `grant`.`role_id` = ? AND `grant`.`object` = ? AND `grant`.`object_name` = ? AND `is_deleted` = ? AND `tenant_id` = ? LIMIT 1"). - WithArgs(grant.RoleID, grant.Object, grant.ObjectName, grant.IsDeleted, grant.TenantID). - WillReturnError(gorm.ErrRecordNotFound) - err = grantTestDb.Delete(grant.TenantID, grant.RoleID, grant.Object, grant.ObjectName, privilege) - assert.Error(t, err) - assert.True(t, common.IsIgnorableError(err)) -} - -func TestGrant_Delete_GetError(t *testing.T) { - var ( - grant *dbmodel.Grant - privilege = "PrivilegeIndexDetail" - err error - ) - grant = &dbmodel.Grant{ - Base: GetBase(), - RoleID: 1, - Object: "Global", - ObjectName: "Col", - Detail: "[[\"admin\",\"PrivilegeIndexDetail\"]]", - } - - mock.ExpectQuery("SELECT * FROM `grant` WHERE `grant`.`role_id` = ? AND `grant`.`object` = ? AND `grant`.`object_name` = ? AND `is_deleted` = ? AND `tenant_id` = ? LIMIT 1"). - WithArgs(grant.RoleID, grant.Object, grant.ObjectName, grant.IsDeleted, grant.TenantID). - WillReturnError(errors.New("test error")) - err = grantTestDb.Delete(grant.TenantID, grant.RoleID, grant.Object, grant.ObjectName, privilege) - assert.Error(t, err) -} - -func TestGrant_Delete_DecodeError(t *testing.T) { - var ( - grant *dbmodel.Grant - privilege = "PrivilegeIndexDetail" - err error - ) - grant = &dbmodel.Grant{ - Base: GetBase(), - RoleID: 1, - Object: "Global", - ObjectName: "Col", - Detail: "[[\"admin\",\"PrivilegeIndexDetail\"]]", - } - - mock.ExpectQuery("SELECT * FROM `grant` WHERE `grant`.`role_id` = ? AND `grant`.`object` = ? AND `grant`.`object_name` = ? AND `is_deleted` = ? AND `tenant_id` = ? LIMIT 1"). - WithArgs(grant.RoleID, grant.Object, grant.ObjectName, grant.IsDeleted, grant.TenantID). - WillReturnRows( - sqlmock.NewRows([]string{"tenant_id", "role_id", "object", "object_name", "detail"}). - AddRow(tenantID, grant.RoleID, grant.Object, grant.ObjectName, "aaa")) - err = grantTestDb.Delete(grant.TenantID, grant.RoleID, grant.Object, grant.ObjectName, privilege) - assert.Error(t, err) -} - -func TestGrant_Delete_Mark(t *testing.T) { - var ( - grant *dbmodel.Grant - privilege = "PrivilegeIndexDetail" - err error - ) - grant = &dbmodel.Grant{ - Base: GetBase(), - RoleID: 1, - Object: "Global", - ObjectName: "Col", - Detail: "[[\"admin\",\"PrivilegeIndexDetail\"]]", - } - - mock.ExpectQuery("SELECT * FROM `grant` WHERE `grant`.`role_id` = ? AND `grant`.`object` = ? AND `grant`.`object_name` = ? AND `is_deleted` = ? AND `tenant_id` = ? LIMIT 1"). - WithArgs(grant.RoleID, grant.Object, grant.ObjectName, grant.IsDeleted, grant.TenantID). - WillReturnRows( - sqlmock.NewRows([]string{"tenant_id", "role_id", "object", "object_name", "detail"}). - AddRow(tenantID, grant.RoleID, grant.Object, grant.ObjectName, grant.Detail)) mock.ExpectBegin() mock.ExpectExec("UPDATE `grant` SET `is_deleted`=?,`updated_at`=? WHERE `grant`.`role_id` = ? AND `grant`.`object` = ? AND `grant`.`object_name` = ? AND `is_deleted` = ? AND `tenant_id` = ?"). WithArgs(true, AnyTime{}, grant.RoleID, grant.Object, grant.ObjectName, false, grant.TenantID). WillReturnResult(sqlmock.NewResult(1, 1)) mock.ExpectCommit() - err = grantTestDb.Delete(grant.TenantID, grant.RoleID, grant.Object, grant.ObjectName, privilege) + err = grantTestDb.Delete(grant.TenantID, grant.RoleID, grant.Object, grant.ObjectName) + mock.MatchExpectationsInOrder(false) assert.NoError(t, err) } -func TestGrant_Delete_Update(t *testing.T) { +func TestGrant_Delete_Error(t *testing.T) { var ( - grant *dbmodel.Grant - privilege = "PrivilegeIndexDetail" - expectDetail = "[[\"admin\",\"PrivilegeLoad\"]]" - err error + grant *dbmodel.Grant + err error ) grant = &dbmodel.Grant{ Base: GetBase(), RoleID: 1, Object: "Global", ObjectName: "Col", - Detail: "[[\"admin\",\"PrivilegeIndexDetail\"],[\"admin\",\"PrivilegeLoad\"]]", } - mock.ExpectQuery("SELECT * FROM `grant` WHERE `grant`.`role_id` = ? AND `grant`.`object` = ? AND `grant`.`object_name` = ? AND `is_deleted` = ? AND `tenant_id` = ? LIMIT 1"). - WithArgs(grant.RoleID, grant.Object, grant.ObjectName, grant.IsDeleted, grant.TenantID). - WillReturnRows( - sqlmock.NewRows([]string{"tenant_id", "role_id", "object", "object_name", "detail"}). - AddRow(tenantID, grant.RoleID, grant.Object, grant.ObjectName, grant.Detail)) mock.ExpectBegin() - mock.ExpectExec("UPDATE `grant` SET `detail`=?,`updated_at`=? WHERE `grant`.`role_id` = ? AND `grant`.`object` = ? AND `grant`.`object_name` = ? AND `is_deleted` = ? AND `tenant_id` = ?"). - WithArgs(expectDetail, AnyTime{}, grant.RoleID, grant.Object, grant.ObjectName, false, grant.TenantID). - WillReturnResult(sqlmock.NewResult(1, 1)) + mock.ExpectExec("UPDATE `grant` SET `is_deleted`=?,`updated_at`=? WHERE `grant`.`role_id` = ? AND `grant`.`object` = ? AND `grant`.`object_name` = ? AND `is_deleted` = ? AND `tenant_id` = ?"). + WithArgs(true, AnyTime{}, grant.RoleID, grant.Object, grant.ObjectName, false, grant.TenantID). + WillReturnError(errors.New("test error")) mock.ExpectCommit() - err = grantTestDb.Delete(grant.TenantID, grant.RoleID, grant.Object, grant.ObjectName, privilege) - assert.NoError(t, err) -} - -func TestGrant_Delete_UpdateError(t *testing.T) { - var ( - grant *dbmodel.Grant - privilege = "PrivilegeIndexDetail" - err error - ) - grant = &dbmodel.Grant{ - Base: GetBase(), - RoleID: 1, - Object: "Global", - ObjectName: "Col", - Detail: "[[\"admin\",\"PrivilegeIndexLoad\"],[\"admin\",\"PrivilegeQuery\"]]", - } - - mock.ExpectQuery("SELECT * FROM `grant` WHERE `grant`.`role_id` = ? AND `grant`.`object` = ? AND `grant`.`object_name` = ? AND `is_deleted` = ? AND `tenant_id` = ? LIMIT 1"). - WithArgs(grant.RoleID, grant.Object, grant.ObjectName, grant.IsDeleted, grant.TenantID). - WillReturnRows( - sqlmock.NewRows([]string{"tenant_id", "role_id", "object", "object_name", "detail"}). - AddRow(tenantID, grant.RoleID, grant.Object, grant.ObjectName, grant.Detail)) - err = grantTestDb.Delete(grant.TenantID, grant.RoleID, grant.Object, grant.ObjectName, privilege) + err = grantTestDb.Delete(grant.TenantID, grant.RoleID, grant.Object, grant.ObjectName) + mock.MatchExpectationsInOrder(false) assert.Error(t, err) - assert.True(t, common.IsIgnorableError(err)) } diff --git a/internal/metastore/db/dao/role.go b/internal/metastore/db/dao/role.go index 4020a5d729..32023d77d5 100644 --- a/internal/metastore/db/dao/role.go +++ b/internal/metastore/db/dao/role.go @@ -16,7 +16,10 @@ func (r *roleDb) GetRoles(tenantID string, name string) ([]*dbmodel.Role, error) roles []*dbmodel.Role err error ) - err = r.db.Model(&dbmodel.Role{}).Where(&dbmodel.Role{Name: name}).Where(dbmodel.GetCommonCondition(tenantID, false)).Find(&roles).Error + err = r.db.Model(&dbmodel.Role{}). + Where(&dbmodel.Role{Name: name}). + Where(dbmodel.GetCommonCondition(tenantID, false)). + Find(&roles).Error if err != nil { log.Error("fail to get roles", zap.String("tenant_id", tenantID), zap.String("name", name), zap.Error(err)) return nil, err @@ -33,7 +36,10 @@ func (r *roleDb) Insert(in *dbmodel.Role) error { } func (r *roleDb) Delete(tenantID string, name string) error { - err := r.db.Model(dbmodel.Role{}).Where(&dbmodel.Role{Name: name}).Where(dbmodel.GetCommonCondition(tenantID, false)).Update("is_deleted", true).Error + err := r.db.Model(dbmodel.Role{}). + Where(&dbmodel.Role{Name: name}). + Where(dbmodel.GetCommonCondition(tenantID, false)). + Update("is_deleted", true).Error if err != nil { log.Error("fail to delete the role", zap.String("tenant_id", tenantID), zap.String("name", name), zap.Error(err)) } diff --git a/internal/metastore/db/dao/user_role.go b/internal/metastore/db/dao/user_role.go index 20a6564d85..e5d1c16426 100644 --- a/internal/metastore/db/dao/user_role.go +++ b/internal/metastore/db/dao/user_role.go @@ -16,7 +16,11 @@ func (u *userRoleDb) GetUserRoles(tenantID string, userID int64, roleID int64) ( userRoles []*dbmodel.UserRole err error ) - err = u.db.Model(&dbmodel.UserRole{}).Where(&dbmodel.UserRole{UserID: userID, RoleID: roleID}).Where(dbmodel.GetCommonCondition(tenantID, false)).Preload("User").Preload("Role").Find(&userRoles).Error + err = u.db.Model(&dbmodel.UserRole{}). + Where(&dbmodel.UserRole{UserID: userID, RoleID: roleID}). + Where(dbmodel.GetCommonCondition(tenantID, false)). + Preload("User").Preload("Role"). + Find(&userRoles).Error if err != nil { log.Error("fail to get user-roles", zap.String("tenant_id", tenantID), zap.Int64("userID", userID), zap.Int64("roleID", roleID), zap.Error(err)) return nil, err @@ -33,7 +37,10 @@ func (u *userRoleDb) Insert(in *dbmodel.UserRole) error { } func (u *userRoleDb) Delete(tenantID string, userID int64, roleID int64) error { - err := u.db.Model(dbmodel.UserRole{}).Where(&dbmodel.UserRole{UserID: userID, RoleID: roleID}).Where(dbmodel.GetCommonCondition(tenantID, false)).Update("is_deleted", true).Error + err := u.db.Model(dbmodel.UserRole{}). + Where(&dbmodel.UserRole{UserID: userID, RoleID: roleID}). + Where(dbmodel.GetCommonCondition(tenantID, false)). + Update("is_deleted", true).Error if err != nil { log.Error("fail to delete the user-role", zap.String("tenant_id", tenantID), zap.Int64("userID", userID), zap.Int64("roleID", roleID), zap.Error(err)) } diff --git a/internal/metastore/db/dao/user_role_test.go b/internal/metastore/db/dao/user_role_test.go index c99beeca43..533330bab4 100644 --- a/internal/metastore/db/dao/user_role_test.go +++ b/internal/metastore/db/dao/user_role_test.go @@ -17,19 +17,16 @@ func TestUserRole_GetUserRoles(t *testing.T) { roleID1 = 10 roleID2 = 20 userRoles []*dbmodel.UserRole - getQuery func() *sqlmock.ExpectedQuery err error ) // mock user and role - getQuery = func() *sqlmock.ExpectedQuery { - return mock.ExpectQuery("SELECT * FROM `user_role` WHERE `is_deleted` = ? AND `tenant_id` = ?"). - WithArgs(false, tenantID) - } - getQuery().WillReturnRows( - sqlmock.NewRows([]string{"tenant_id", "user_id", "role_id"}). - AddRow(tenantID, userID1, roleID1). - AddRow(tenantID, userID2, roleID2)) + mock.ExpectQuery("SELECT * FROM `user_role` WHERE `is_deleted` = ? AND `tenant_id` = ?"). + WithArgs(false, tenantID). + WillReturnRows( + sqlmock.NewRows([]string{"tenant_id", "user_id", "role_id"}). + AddRow(tenantID, userID1, roleID1). + AddRow(tenantID, userID2, roleID2)) mock.ExpectQuery("SELECT * FROM `role` WHERE `role`.`id` IN (?,?)"). WithArgs(roleID1, roleID2). @@ -46,13 +43,17 @@ func TestUserRole_GetUserRoles(t *testing.T) { AddRow(userID2, tenantID, "fo2")) userRoles, err = userRoleTestDb.GetUserRoles(tenantID, 0, 0) + mock.MatchExpectationsInOrder(false) assert.NoError(t, err) assert.Equal(t, 2, len(userRoles)) assert.Equal(t, "foo1", userRoles[0].Role.Name) assert.Equal(t, "fo1", userRoles[0].User.Username) - getQuery().WillReturnError(errors.New("test error")) + mock.ExpectQuery("SELECT * FROM `user_role` WHERE `is_deleted` = ? AND `tenant_id` = ?"). + WithArgs(false, tenantID). + WillReturnError(errors.New("test error")) _, err = userRoleTestDb.GetUserRoles(tenantID, 0, 0) + mock.MatchExpectationsInOrder(false) assert.Error(t, err) } @@ -84,6 +85,7 @@ func TestUserRole_GetUserRolesWithUserID(t *testing.T) { AddRow(userID1, tenantID, "fo1")) userRoles, err = userRoleTestDb.GetUserRoles(tenantID, int64(userID1), 0) + mock.MatchExpectationsInOrder(false) assert.NoError(t, err) assert.Equal(t, 2, len(userRoles)) assert.Equal(t, "foo2", userRoles[1].Role.Name) @@ -118,6 +120,7 @@ func TestUserRole_GetUserRolesWithRoleID(t *testing.T) { AddRow(userID2, tenantID, "fo2")) userRoles, err = userRoleTestDb.GetUserRoles(tenantID, 0, int64(roleID1)) + mock.MatchExpectationsInOrder(false) assert.NoError(t, err) assert.Equal(t, 2, len(userRoles)) assert.Equal(t, "foo1", userRoles[0].Role.Name) @@ -141,6 +144,7 @@ func TestUserRole_Insert(t *testing.T) { WillReturnResult(sqlmock.NewResult(1, 1)) mock.ExpectCommit() err = userRoleTestDb.Insert(userRole) + mock.MatchExpectationsInOrder(false) assert.NoError(t, err) } @@ -161,6 +165,7 @@ func TestUserRole_InsertError(t *testing.T) { WillReturnError(errors.New("test error")) mock.ExpectRollback() err = userRoleTestDb.Insert(userRole) + mock.MatchExpectationsInOrder(false) assert.Error(t, err) } @@ -183,11 +188,13 @@ func TestUserRole_Delete(t *testing.T) { getExec().WillReturnResult(sqlmock.NewResult(1, 1)) mock.ExpectCommit() err = userRoleTestDb.Delete(userRole.TenantID, userRole.UserID, userRole.RoleID) + mock.MatchExpectationsInOrder(false) assert.NoError(t, err) mock.ExpectBegin() getExec().WillReturnError(errors.New("test error")) mock.ExpectRollback() err = userRoleTestDb.Delete(userRole.TenantID, userRole.UserID, userRole.RoleID) + mock.MatchExpectationsInOrder(false) assert.Error(t, err) } diff --git a/internal/metastore/db/dbmodel/common.go b/internal/metastore/db/dbmodel/common.go index 682abbd7c9..60c5dd4b55 100644 --- a/internal/metastore/db/dbmodel/common.go +++ b/internal/metastore/db/dbmodel/common.go @@ -15,6 +15,7 @@ type IMetaDomain interface { RoleDb(ctx context.Context) IRoleDb UserRoleDb(ctx context.Context) IUserRoleDb GrantDb(ctx context.Context) IGrantDb + GrantIDDb(ctx context.Context) IGrantIDDb } type ITransaction interface { diff --git a/internal/metastore/db/dbmodel/grant.go b/internal/metastore/db/dbmodel/grant.go index db0b0f9afb..ffb9e6af30 100644 --- a/internal/metastore/db/dbmodel/grant.go +++ b/internal/metastore/db/dbmodel/grant.go @@ -1,19 +1,11 @@ package dbmodel -import ( - "encoding/json" - "fmt" - - "github.com/milvus-io/milvus/internal/common" -) - type Grant struct { Base RoleID int64 `gorm:"role_id"` Role Role `gorm:"foreignKey:RoleID"` Object string `gorm:"object"` ObjectName string `gorm:"object_name"` - Detail string `gorm:"detail"` } func (g *Grant) TableName() string { @@ -24,78 +16,5 @@ func (g *Grant) TableName() string { type IGrantDb interface { GetGrants(tenantID string, roleID int64, object string, objectName string) ([]*Grant, error) Insert(in *Grant) error - Delete(tenantID string, roleID int64, object string, objectName string, privilege string) error -} - -func EncodeGrantDetail(detail string, grantor string, privilege string, isAdd bool) (string, error) { - var ( - grant = []string{grantor, privilege} - resBytes []byte - originGrants [][]string - index = -1 - err error - - handleGrant = func(grants [][]string) (string, error) { - if resBytes, err = json.Marshal(grants); err != nil { - return "", err - } - return string(resBytes), nil - } - ) - - if detail == "" { - if !isAdd { - return "", common.NewIgnorableError(fmt.Errorf("the empty detail can't be remove")) - } - return handleGrant(append(originGrants, grant)) - } - if originGrants, err = DecodeGrantDetail(detail); err != nil { - return "", err - } - - for i, origin := range originGrants { - if origin[1] == privilege { - index = i - break - } - } - if isAdd { - if index != -1 { - return detail, common.NewIgnorableError(fmt.Errorf("the grant[%s-%s] is existed", grantor, privilege)) - } - return handleGrant(append(originGrants, grant)) - } - if index == -1 { - return detail, common.NewIgnorableError(fmt.Errorf("the grant[%s-%s] isn't existed", grantor, privilege)) - } - if len(originGrants) == 1 { - return "", nil - } - return handleGrant(append(originGrants[:index], originGrants[index+1:]...)) -} - -func EncodeGrantDetailForString(originDetail string, operateDetail string, isAdd bool) (string, error) { - var ( - operateGrant [][]string - err error - ) - - if operateGrant, err = DecodeGrantDetail(operateDetail); err != nil { - return "", err - } - if len(operateGrant) != 1 || len(operateGrant[0]) != 2 { - return "", fmt.Errorf("invalid operateDetail: [%s], decode result: %+v", operateDetail, operateGrant) - } - return EncodeGrantDetail(originDetail, operateGrant[0][0], operateGrant[0][1], isAdd) -} - -func DecodeGrantDetail(detail string) ([][]string, error) { - var ( - grants [][]string - err error - ) - if err = json.Unmarshal([]byte(detail), &grants); err != nil { - return grants, err - } - return grants, nil + Delete(tenantID string, roleID int64, object string, objectName string) error } diff --git a/internal/metastore/db/dbmodel/grant_id.go b/internal/metastore/db/dbmodel/grant_id.go new file mode 100644 index 0000000000..d67e00eba7 --- /dev/null +++ b/internal/metastore/db/dbmodel/grant_id.go @@ -0,0 +1,21 @@ +package dbmodel + +type GrantID struct { + Base + GrantID int64 `gorm:"grant_id"` + Grant Grant `gorm:"foreignKey:GrantID"` + Privilege string `gorm:"privilege"` + GrantorID int64 `gorm:"grantor_id"` + Grantor User `gorm:"foreignKey:GrantorID"` +} + +func (g *GrantID) TableName() string { + return "grant_id" +} + +//go:generate mockery --name=IGrantIDDb +type IGrantIDDb interface { + GetGrantIDs(tenantID string, grantID int64, privilege string, preloadGrant bool, preloadGrantor bool) ([]*GrantID, error) + Insert(in *GrantID) error + Delete(tenantID string, grantID int64, privilege string) error +} diff --git a/internal/metastore/db/dbmodel/mocks/IGrantDb.go b/internal/metastore/db/dbmodel/mocks/IGrantDb.go index 861d923ba2..672d9895cf 100644 --- a/internal/metastore/db/dbmodel/mocks/IGrantDb.go +++ b/internal/metastore/db/dbmodel/mocks/IGrantDb.go @@ -12,13 +12,13 @@ type IGrantDb struct { mock.Mock } -// Delete provides a mock function with given fields: tenantID, roleID, object, objectName, privilege -func (_m *IGrantDb) Delete(tenantID string, roleID int64, object string, objectName string, privilege string) error { - ret := _m.Called(tenantID, roleID, object, objectName, privilege) +// Delete provides a mock function with given fields: tenantID, roleID, object, objectName +func (_m *IGrantDb) Delete(tenantID string, roleID int64, object string, objectName string) error { + ret := _m.Called(tenantID, roleID, object, objectName) var r0 error - if rf, ok := ret.Get(0).(func(string, int64, string, string, string) error); ok { - r0 = rf(tenantID, roleID, object, objectName, privilege) + if rf, ok := ret.Get(0).(func(string, int64, string, string) error); ok { + r0 = rf(tenantID, roleID, object, objectName) } else { r0 = ret.Error(0) } diff --git a/internal/metastore/db/dbmodel/mocks/IGrantIDDb.go b/internal/metastore/db/dbmodel/mocks/IGrantIDDb.go new file mode 100644 index 0000000000..8411ebb982 --- /dev/null +++ b/internal/metastore/db/dbmodel/mocks/IGrantIDDb.go @@ -0,0 +1,79 @@ +// Code generated by mockery v2.14.0. DO NOT EDIT. + +package mocks + +import ( + dbmodel "github.com/milvus-io/milvus/internal/metastore/db/dbmodel" + mock "github.com/stretchr/testify/mock" +) + +// IGrantIDDb is an autogenerated mock type for the IGrantIDDb type +type IGrantIDDb struct { + mock.Mock +} + +// Delete provides a mock function with given fields: tenantID, grantID, privilege +func (_m *IGrantIDDb) Delete(tenantID string, grantID int64, privilege string) error { + ret := _m.Called(tenantID, grantID, privilege) + + var r0 error + if rf, ok := ret.Get(0).(func(string, int64, string) error); ok { + r0 = rf(tenantID, grantID, privilege) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// GetGrantIDs provides a mock function with given fields: tenantID, grantID, privilege, preloadGrant, preloadGrantor +func (_m *IGrantIDDb) GetGrantIDs(tenantID string, grantID int64, privilege string, preloadGrant bool, preloadGrantor bool) ([]*dbmodel.GrantID, error) { + ret := _m.Called(tenantID, grantID, privilege, preloadGrant, preloadGrantor) + + var r0 []*dbmodel.GrantID + if rf, ok := ret.Get(0).(func(string, int64, string, bool, bool) []*dbmodel.GrantID); ok { + r0 = rf(tenantID, grantID, privilege, preloadGrant, preloadGrantor) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]*dbmodel.GrantID) + } + } + + var r1 error + if rf, ok := ret.Get(1).(func(string, int64, string, bool, bool) error); ok { + r1 = rf(tenantID, grantID, privilege, preloadGrant, preloadGrantor) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// Insert provides a mock function with given fields: in +func (_m *IGrantIDDb) Insert(in *dbmodel.GrantID) error { + ret := _m.Called(in) + + var r0 error + if rf, ok := ret.Get(0).(func(*dbmodel.GrantID) error); ok { + r0 = rf(in) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +type mockConstructorTestingTNewIGrantIDDb interface { + mock.TestingT + Cleanup(func()) +} + +// NewIGrantIDDb creates a new instance of IGrantIDDb. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +func NewIGrantIDDb(t mockConstructorTestingTNewIGrantIDDb) *IGrantIDDb { + mock := &IGrantIDDb{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} diff --git a/internal/metastore/db/dbmodel/mocks/IMetaDomain.go b/internal/metastore/db/dbmodel/mocks/IMetaDomain.go index 3ca661a81b..12a3653cfd 100644 --- a/internal/metastore/db/dbmodel/mocks/IMetaDomain.go +++ b/internal/metastore/db/dbmodel/mocks/IMetaDomain.go @@ -94,6 +94,22 @@ func (_m *IMetaDomain) GrantDb(ctx context.Context) dbmodel.IGrantDb { return r0 } +// GrantIDDb provides a mock function with given fields: ctx +func (_m *IMetaDomain) GrantIDDb(ctx context.Context) dbmodel.IGrantIDDb { + ret := _m.Called(ctx) + + var r0 dbmodel.IGrantIDDb + if rf, ok := ret.Get(0).(func(context.Context) dbmodel.IGrantIDDb); ok { + r0 = rf(ctx) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(dbmodel.IGrantIDDb) + } + } + + return r0 +} + // IndexDb provides a mock function with given fields: ctx func (_m *IMetaDomain) IndexDb(ctx context.Context) dbmodel.IIndexDb { ret := _m.Called(ctx) diff --git a/internal/metastore/db/rootcoord/table_catalog.go b/internal/metastore/db/rootcoord/table_catalog.go index 725f9b83f2..a50da1262c 100644 --- a/internal/metastore/db/rootcoord/table_catalog.go +++ b/internal/metastore/db/rootcoord/table_catalog.go @@ -6,10 +6,10 @@ import ( "fmt" "runtime" - "github.com/milvus-io/milvus/internal/common" - "github.com/milvus-io/milvus/internal/util" + "github.com/milvus-io/milvus/internal/common" + "github.com/milvus-io/milvus/internal/log" "github.com/milvus-io/milvus/internal/metastore/db/dbmodel" "github.com/milvus-io/milvus/internal/metastore/model" @@ -709,34 +709,94 @@ func (tc *Catalog) ListUser(ctx context.Context, tenant string, entity *milvuspb return results, nil } -func (tc *Catalog) AlterGrant(ctx context.Context, tenant string, entity *milvuspb.GrantEntity, operateType milvuspb.OperatePrivilegeType) error { +func (tc Catalog) grant(ctx context.Context, tenant string, entity *milvuspb.GrantEntity) error { var ( - roleID int64 - detail string - err error + roleID int64 + grants []*dbmodel.Grant + grantID int64 + grantIDObjs []*dbmodel.GrantID + err error + ) + if roleID, err = tc.GetRoleIDByName(ctx, tenant, entity.Role.Name); err != nil { + return err + } + + if grants, err = tc.metaDomain.GrantDb(ctx).GetGrants(tenant, roleID, entity.Object.Name, entity.ObjectName); err != nil { + return err + } + if len(grants) == 0 { + var grant = &dbmodel.Grant{ + Base: dbmodel.Base{TenantID: tenant}, + RoleID: roleID, + Object: entity.Object.Name, + ObjectName: entity.ObjectName, + } + if err = tc.metaDomain.GrantDb(ctx).Insert(grant); err != nil { + return err + } + log.Debug("grant id", zap.Int64("id", grant.ID)) + grantID = grant.ID + } else { + grantID = grants[0].ID + } + if grantIDObjs, err = tc.metaDomain.GrantIDDb(ctx).GetGrantIDs(tenant, grantID, entity.Grantor.Privilege.Name, false, false); err != nil { + return err + } + if len(grantIDObjs) > 0 { + log.Warn("the grant id has existed", zap.Any("entity", entity)) + return common.NewIgnorableError(fmt.Errorf("the privilege [%s] has been grantd for the role [%s]", entity.Grantor.Privilege.Name, entity.Role.Name)) + } + var user *dbmodel.User + if user, err = tc.metaDomain.UserDb(ctx).GetByUsername(tenant, entity.Grantor.User.Name); err != nil { + return err + } + return tc.metaDomain.GrantIDDb(ctx).Insert(&dbmodel.GrantID{ + Base: dbmodel.Base{TenantID: tenant}, + GrantID: grantID, + Privilege: entity.Grantor.Privilege.Name, + GrantorID: user.ID, + }) +} + +func (tc Catalog) revoke(ctx context.Context, tenant string, entity *milvuspb.GrantEntity) error { + var ( + roleID int64 + grants []*dbmodel.Grant + grantID int64 + grantIDObjs []*dbmodel.GrantID + err error ) if roleID, err = tc.GetRoleIDByName(ctx, tenant, entity.Role.Name); err != nil { return err } + + if grants, err = tc.metaDomain.GrantDb(ctx).GetGrants(tenant, roleID, entity.Object.Name, entity.ObjectName); err != nil { + return err + } + if len(grants) == 0 { + log.Warn("the grant isn't existed", zap.Any("entity", entity)) + return common.NewIgnorableError(fmt.Errorf("the privilege [%s] isn't grantd for the role [%s]", entity.Grantor.Privilege.Name, entity.Role.Name)) + } + grantID = grants[0].ID + if grantIDObjs, err = tc.metaDomain.GrantIDDb(ctx).GetGrantIDs(tenant, grantID, entity.Grantor.Privilege.Name, false, false); err != nil { + return err + } + if len(grantIDObjs) == 0 { + log.Error("the grant-id isn't existed", zap.Any("entity", entity)) + return common.NewIgnorableError(fmt.Errorf("the privilege [%s] isn't grantd for the role [%s]", entity.Grantor.Privilege.Name, entity.Role.Name)) + } + return tc.metaDomain.GrantIDDb(ctx).Delete(tenant, grantID, entity.Grantor.Privilege.Name) +} + +func (tc *Catalog) AlterGrant(ctx context.Context, tenant string, entity *milvuspb.GrantEntity, operateType milvuspb.OperatePrivilegeType) error { switch operateType { - case milvuspb.OperatePrivilegeType_Revoke: - return tc.metaDomain.GrantDb(ctx). - Delete(tenant, roleID, entity.Object.Name, entity.ObjectName, entity.Grantor.Privilege.Name) case milvuspb.OperatePrivilegeType_Grant: - if detail, err = dbmodel.EncodeGrantDetail("", entity.Grantor.User.Name, entity.Grantor.Privilege.Name, true); err != nil { - log.Error("fail to encode grant detail", zap.String("tenant", tenant), zap.Any("entity", entity), zap.Error(err)) - return err - } - return tc.metaDomain.GrantDb(ctx).Insert(&dbmodel.Grant{ - Base: dbmodel.Base{TenantID: tenant}, - RoleID: roleID, - Object: entity.Object.Name, - ObjectName: entity.ObjectName, - Detail: detail, - }) + return tc.grant(ctx, tenant, entity) + case milvuspb.OperatePrivilegeType_Revoke: + return tc.revoke(ctx, tenant, entity) default: - err = fmt.Errorf("invalid operate type: %d", operateType) + err := fmt.Errorf("invalid operate type: %d", operateType) log.Error("error: ", zap.Error(err)) return err } @@ -749,7 +809,8 @@ func (tc *Catalog) ListGrant(ctx context.Context, tenant string, entity *milvusp objectName string grants []*dbmodel.Grant grantEntities []*milvuspb.GrantEntity - details [][]string + grantIDDb dbmodel.IGrantIDDb + grantIDs []*dbmodel.GrantID privilegeName string err error ) @@ -764,18 +825,14 @@ func (tc *Catalog) ListGrant(ctx context.Context, tenant string, entity *milvusp if grants, err = tc.metaDomain.GrantDb(ctx).GetGrants(tenant, roleID, object, objectName); err != nil { return nil, err } + grantIDDb = tc.metaDomain.GrantIDDb(ctx) for _, grant := range grants { - if details, err = dbmodel.DecodeGrantDetail(grant.Detail); err != nil { - log.Error("fail to decode grant detail", zap.Any("detail", grant.Detail), zap.Error(err)) + if grantIDs, err = grantIDDb.GetGrantIDs(tenant, grant.ID, "", false, true); err != nil { return nil, err } - for _, detail := range details { - if len(detail) != 2 { - log.Error("invalid operateDetail", zap.Any("detail", detail)) - return nil, fmt.Errorf("invalid operateDetail: [%s], decode result: %+v", grant.Detail, details) - } - privilegeName = util.PrivilegeNameForAPI(detail[1]) - if detail[1] == util.AnyWord { + for _, grantID := range grantIDs { + privilegeName = util.PrivilegeNameForAPI(grantID.Privilege) + if grantID.Privilege == util.AnyWord { privilegeName = util.AnyWord } grantEntities = append(grantEntities, &milvuspb.GrantEntity{ @@ -783,7 +840,7 @@ func (tc *Catalog) ListGrant(ctx context.Context, tenant string, entity *milvusp Object: &milvuspb.ObjectEntity{Name: grant.Object}, ObjectName: grant.ObjectName, Grantor: &milvuspb.GrantorEntity{ - User: &milvuspb.UserEntity{Name: detail[0]}, + User: &milvuspb.UserEntity{Name: grantID.Grantor.Username}, Privilege: &milvuspb.PrivilegeEntity{Name: privilegeName}, }, }) @@ -795,28 +852,38 @@ func (tc *Catalog) ListGrant(ctx context.Context, tenant string, entity *milvusp return grantEntities, nil } +func (tc *Catalog) DeleteGrant(ctx context.Context, tenant string, role *milvuspb.RoleEntity) error { + var ( + roleID int64 + err error + ) + + if roleID, err = tc.GetRoleIDByName(ctx, tenant, role.Name); err != nil { + log.Error("fail to get roleID by the role name", zap.String("role_name", role.Name), zap.Error(err)) + return err + } + return tc.metaDomain.GrantDb(ctx).Delete(tenant, roleID, "", "") +} + func (tc *Catalog) ListPolicy(ctx context.Context, tenant string) ([]string, error) { var ( - grants []*dbmodel.Grant - details [][]string - policies []string - err error + grants []*dbmodel.Grant + grantIDDb dbmodel.IGrantIDDb + grantIDs []*dbmodel.GrantID + policies []string + err error ) if grants, err = tc.metaDomain.GrantDb(ctx).GetGrants(tenant, 0, "", ""); err != nil { return nil, err } + grantIDDb = tc.metaDomain.GrantIDDb(ctx) for _, grant := range grants { - if details, err = dbmodel.DecodeGrantDetail(grant.Detail); err != nil { - log.Error("fail to decode grant detail", zap.Any("detail", grant.Detail), zap.Error(err)) + if grantIDs, err = grantIDDb.GetGrantIDs(tenant, grant.ID, "", false, false); err != nil { return nil, err } - for _, detail := range details { - if len(detail) != 2 { - log.Error("invalid operateDetail", zap.String("tenant", tenant), zap.Strings("detail", detail)) - return nil, fmt.Errorf("invalid operateDetail: %+v", detail) - } + for _, grantID := range grantIDs { policies = append(policies, - funcutil.PolicyForPrivilege(grant.Role.Name, grant.Object, grant.ObjectName, detail[1])) + funcutil.PolicyForPrivilege(grant.Role.Name, grant.Object, grant.ObjectName, grantID.Privilege)) } } diff --git a/internal/metastore/db/rootcoord/table_catalog_test.go b/internal/metastore/db/rootcoord/table_catalog_test.go index 6d462f7d55..15547a87b1 100644 --- a/internal/metastore/db/rootcoord/table_catalog_test.go +++ b/internal/metastore/db/rootcoord/table_catalog_test.go @@ -55,6 +55,7 @@ var ( roleDbMock *mocks.IRoleDb userRoleDbMock *mocks.IUserRoleDb grantDbMock *mocks.IGrantDb + grantIDDbMock *mocks.IGrantIDDb mockCatalog *Catalog ) @@ -74,6 +75,7 @@ func TestMain(m *testing.M) { roleDbMock = &mocks.IRoleDb{} userRoleDbMock = &mocks.IUserRoleDb{} grantDbMock = &mocks.IGrantDb{} + grantIDDbMock = &mocks.IGrantIDDb{} metaDomainMock = &mocks.IMetaDomain{} metaDomainMock.On("CollectionDb", ctx).Return(collDbMock) @@ -87,6 +89,7 @@ func TestMain(m *testing.M) { metaDomainMock.On("RoleDb", ctx).Return(roleDbMock) metaDomainMock.On("UserRoleDb", ctx).Return(userRoleDbMock) metaDomainMock.On("GrantDb", ctx).Return(grantDbMock) + metaDomainMock.On("GrantIDDb", ctx).Return(grantIDDbMock) mockCatalog = mockMetaCatalog(metaDomainMock) @@ -1525,61 +1528,149 @@ func TestTableCatalog_ListUser_GetUserRolesError(t *testing.T) { require.Error(t, err) } -func TestTableCatalog_AlterPrivilege_Revoke(t *testing.T) { +func TestTableCatalog_AlterGrant_Revoke(t *testing.T) { var ( - grant *milvuspb.GrantEntity - err error + roleName = "foo" + roleID int64 = 1 + object = "Collection" + objectName = "col1" + grantID int64 = 10 + username = "fo" + privilege = "PrivilegeLoad" + grant *milvuspb.GrantEntity + err error ) grant = &milvuspb.GrantEntity{ - Role: &milvuspb.RoleEntity{Name: "foo"}, - Object: &milvuspb.ObjectEntity{Name: "Collection"}, - ObjectName: "col1", + Role: &milvuspb.RoleEntity{Name: roleName}, + Object: &milvuspb.ObjectEntity{Name: object}, + ObjectName: objectName, Grantor: &milvuspb.GrantorEntity{ - User: &milvuspb.UserEntity{Name: "foo"}, - Privilege: &milvuspb.PrivilegeEntity{Name: "PrivilegeLoad"}, + User: &milvuspb.UserEntity{Name: username}, + Privilege: &milvuspb.PrivilegeEntity{Name: privilege}, }, } - roleDbMock.On("GetRoles", tenantID, grant.Role.Name).Return([]*dbmodel.Role{{Base: dbmodel.Base{ID: 1}}}, nil).Once() - grantDbMock.On("Delete", tenantID, int64(1), grant.Object.Name, grant.ObjectName, grant.Grantor.Privilege.Name).Return(nil).Once() + roleDbMock.On("GetRoles", tenantID, roleName).Return([]*dbmodel.Role{{Base: dbmodel.Base{ID: roleID}}}, nil).Once() + grantDbMock.On("GetGrants", tenantID, roleID, object, objectName).Return([]*dbmodel.Grant{{Base: dbmodel.Base{ID: grantID}}}, nil).Once() + grantIDDbMock.On("GetGrantIDs", tenantID, grantID, privilege, mock.Anything, mock.Anything).Return([]*dbmodel.GrantID{{}}, nil).Once() + grantIDDbMock.On("Delete", tenantID, grantID, privilege).Return(nil).Once() err = mockCatalog.AlterGrant(ctx, tenantID, grant, milvuspb.OperatePrivilegeType_Revoke) require.NoError(t, err) + + roleDbMock.On("GetRoles", tenantID, roleName).Return([]*dbmodel.Role{{Base: dbmodel.Base{ID: roleID}}}, nil).Once() + grantDbMock.On("GetGrants", tenantID, roleID, object, objectName).Return(nil, errors.New("test error")).Once() + err = mockCatalog.AlterGrant(ctx, tenantID, grant, milvuspb.OperatePrivilegeType_Revoke) + require.Error(t, err) + + roleDbMock.On("GetRoles", tenantID, roleName).Return([]*dbmodel.Role{{Base: dbmodel.Base{ID: roleID}}}, nil).Once() + grantDbMock.On("GetGrants", tenantID, roleID, object, objectName).Return(nil, nil).Once() + err = mockCatalog.AlterGrant(ctx, tenantID, grant, milvuspb.OperatePrivilegeType_Revoke) + require.Error(t, err) + require.True(t, common.IsIgnorableError(err)) + + roleDbMock.On("GetRoles", tenantID, roleName).Return([]*dbmodel.Role{{Base: dbmodel.Base{ID: roleID}}}, nil).Once() + grantDbMock.On("GetGrants", tenantID, roleID, object, objectName).Return([]*dbmodel.Grant{{Base: dbmodel.Base{ID: grantID}}}, nil).Once() + grantIDDbMock.On("GetGrantIDs", tenantID, grantID, privilege, mock.Anything, mock.Anything).Return(nil, errors.New("test error")).Once() + err = mockCatalog.AlterGrant(ctx, tenantID, grant, milvuspb.OperatePrivilegeType_Revoke) + require.Error(t, err) + + roleDbMock.On("GetRoles", tenantID, roleName).Return([]*dbmodel.Role{{Base: dbmodel.Base{ID: roleID}}}, nil).Once() + grantDbMock.On("GetGrants", tenantID, roleID, object, objectName).Return([]*dbmodel.Grant{{Base: dbmodel.Base{ID: grantID}}}, nil).Once() + grantIDDbMock.On("GetGrantIDs", tenantID, grantID, privilege, mock.Anything, mock.Anything).Return(nil, nil).Once() + err = mockCatalog.AlterGrant(ctx, tenantID, grant, milvuspb.OperatePrivilegeType_Revoke) + require.Error(t, err) + require.True(t, common.IsIgnorableError(err)) + + roleDbMock.On("GetRoles", tenantID, roleName).Return([]*dbmodel.Role{{Base: dbmodel.Base{ID: roleID}}}, nil).Once() + grantDbMock.On("GetGrants", tenantID, roleID, object, objectName).Return([]*dbmodel.Grant{{Base: dbmodel.Base{ID: grantID}}}, nil).Once() + grantIDDbMock.On("GetGrantIDs", tenantID, grantID, privilege, mock.Anything, mock.Anything).Return([]*dbmodel.GrantID{{}}, nil).Once() + grantIDDbMock.On("Delete", tenantID, grantID, privilege).Return(errors.New("test error")).Once() + err = mockCatalog.AlterGrant(ctx, tenantID, grant, milvuspb.OperatePrivilegeType_Revoke) + require.Error(t, err) } -func TestTableCatalog_AlterPrivilege_Grant(t *testing.T) { +func TestTableCatalog_AlterGrant_Grant(t *testing.T) { var ( - grant *milvuspb.GrantEntity - err error + roleName = "foo" + roleID int64 = 1 + object = "Collection" + objectName = "col1" + grantID int64 = 10 + username = "fo" + userID int64 = 100 + privilege = "PrivilegeLoad" + grant *milvuspb.GrantEntity + err error ) grant = &milvuspb.GrantEntity{ - Role: &milvuspb.RoleEntity{Name: "foo"}, - Object: &milvuspb.ObjectEntity{Name: "Collection"}, - ObjectName: "col1", + Role: &milvuspb.RoleEntity{Name: roleName}, + Object: &milvuspb.ObjectEntity{Name: object}, + ObjectName: objectName, Grantor: &milvuspb.GrantorEntity{ - User: &milvuspb.UserEntity{Name: "foo"}, - Privilege: &milvuspb.PrivilegeEntity{Name: "PrivilegeLoad"}, + User: &milvuspb.UserEntity{Name: username}, + Privilege: &milvuspb.PrivilegeEntity{Name: privilege}, }, } - roleDbMock.On("GetRoles", tenantID, grant.Role.Name).Return([]*dbmodel.Role{{Base: dbmodel.Base{ID: 1}}}, nil).Once() - grantDbMock.On("Insert", mock.Anything).Return(nil).Once() - + roleDbMock.On("GetRoles", tenantID, roleName).Return([]*dbmodel.Role{{Base: dbmodel.Base{ID: roleID}}}, nil).Once() + grantDbMock.On("GetGrants", tenantID, roleID, object, objectName).Return([]*dbmodel.Grant{{Base: dbmodel.Base{ID: grantID}}}, nil).Once() + grantIDDbMock.On("GetGrantIDs", tenantID, grantID, privilege, mock.Anything, mock.Anything).Return([]*dbmodel.GrantID{}, nil).Once() + userDbMock.On("GetByUsername", tenantID, username).Return(&dbmodel.User{ID: userID}, nil).Once() + grantIDDbMock.On("Insert", mock.Anything).Return(nil).Once() err = mockCatalog.AlterGrant(ctx, tenantID, grant, milvuspb.OperatePrivilegeType_Grant) require.NoError(t, err) + + roleDbMock.On("GetRoles", tenantID, roleName).Return([]*dbmodel.Role{{Base: dbmodel.Base{ID: roleID}}}, nil).Once() + grantDbMock.On("GetGrants", tenantID, roleID, object, objectName).Return(nil, nil).Once() + grantDbMock.On("Insert", mock.Anything).Return(nil).Once() + grantIDDbMock.On("GetGrantIDs", tenantID, mock.Anything, privilege, mock.Anything, mock.Anything).Return([]*dbmodel.GrantID{}, nil).Once() + userDbMock.On("GetByUsername", tenantID, username).Return(&dbmodel.User{ID: userID}, nil).Once() + grantIDDbMock.On("Insert", mock.Anything).Return(nil).Once() + err = mockCatalog.AlterGrant(ctx, tenantID, grant, milvuspb.OperatePrivilegeType_Grant) + require.NoError(t, err) + + roleDbMock.On("GetRoles", tenantID, roleName).Return([]*dbmodel.Role{{Base: dbmodel.Base{ID: roleID}}}, nil).Once() + grantDbMock.On("GetGrants", tenantID, roleID, object, objectName).Return([]*dbmodel.Grant{{Base: dbmodel.Base{ID: grantID}}}, nil).Once() + grantIDDbMock.On("GetGrantIDs", tenantID, grantID, privilege, mock.Anything, mock.Anything).Return(nil, errors.New("test error")).Once() + err = mockCatalog.AlterGrant(ctx, tenantID, grant, milvuspb.OperatePrivilegeType_Grant) + require.Error(t, err) + + roleDbMock.On("GetRoles", tenantID, roleName).Return([]*dbmodel.Role{{Base: dbmodel.Base{ID: roleID}}}, nil).Once() + grantDbMock.On("GetGrants", tenantID, roleID, object, objectName).Return([]*dbmodel.Grant{{Base: dbmodel.Base{ID: grantID}}}, nil).Once() + grantIDDbMock.On("GetGrantIDs", tenantID, grantID, privilege, mock.Anything, mock.Anything).Return([]*dbmodel.GrantID{{}}, nil).Once() + err = mockCatalog.AlterGrant(ctx, tenantID, grant, milvuspb.OperatePrivilegeType_Grant) + require.Error(t, err) + require.True(t, common.IsIgnorableError(err)) + + roleDbMock.On("GetRoles", tenantID, roleName).Return([]*dbmodel.Role{{Base: dbmodel.Base{ID: roleID}}}, nil).Once() + grantDbMock.On("GetGrants", tenantID, roleID, object, objectName).Return([]*dbmodel.Grant{{Base: dbmodel.Base{ID: grantID}}}, nil).Once() + grantIDDbMock.On("GetGrantIDs", tenantID, grantID, privilege, mock.Anything, mock.Anything).Return([]*dbmodel.GrantID{}, nil).Once() + userDbMock.On("GetByUsername", tenantID, username).Return(nil, errors.New("test error")).Once() + err = mockCatalog.AlterGrant(ctx, tenantID, grant, milvuspb.OperatePrivilegeType_Grant) + require.Error(t, err) + + roleDbMock.On("GetRoles", tenantID, roleName).Return([]*dbmodel.Role{{Base: dbmodel.Base{ID: roleID}}}, nil).Once() + grantDbMock.On("GetGrants", tenantID, roleID, object, objectName).Return([]*dbmodel.Grant{{Base: dbmodel.Base{ID: grantID}}}, nil).Once() + grantIDDbMock.On("GetGrantIDs", tenantID, grantID, privilege, mock.Anything, mock.Anything).Return([]*dbmodel.GrantID{}, nil).Once() + userDbMock.On("GetByUsername", tenantID, username).Return(&dbmodel.User{ID: userID}, nil).Once() + grantIDDbMock.On("Insert", mock.Anything).Return(errors.New("test error")).Once() + err = mockCatalog.AlterGrant(ctx, tenantID, grant, milvuspb.OperatePrivilegeType_Grant) + require.Error(t, err) } -func TestTableCatalog_AlterPrivilege_InvalidType(t *testing.T) { +func TestTableCatalog_AlterGrant_InvalidType(t *testing.T) { var ( roleName = "foo" err error ) - roleDbMock.On("GetRoles", tenantID, roleName).Return([]*dbmodel.Role{{Base: dbmodel.Base{ID: 1}}}, nil).Once() err = mockCatalog.AlterGrant(ctx, tenantID, &milvuspb.GrantEntity{Role: &milvuspb.RoleEntity{Name: roleName}}, 100) require.Error(t, err) } func TestTableCatalog_ListGrant(t *testing.T) { var ( + roleID int64 = 1 + grantID int64 = 10 grant *milvuspb.GrantEntity grants []*dbmodel.Grant entites []*milvuspb.GrantEntity @@ -1593,18 +1684,33 @@ func TestTableCatalog_ListGrant(t *testing.T) { } grants = []*dbmodel.Grant{ { + Base: dbmodel.Base{ID: grantID}, Role: dbmodel.Role{Name: "foo"}, Object: "Collection", ObjectName: "col1", - Detail: "[[\"admin\",\"PrivilegeIndexDetail\"],[\"admin\",\"PrivilegeLoad\"],[\"admin\",\"*\"]]", }, } - roleDbMock.On("GetRoles", tenantID, grant.Role.Name).Return([]*dbmodel.Role{{Base: dbmodel.Base{ID: 1}}}, nil).Once() - grantDbMock.On("GetGrants", tenantID, int64(1), grant.Object.Name, grant.ObjectName).Return(grants, nil).Once() + roleDbMock.On("GetRoles", tenantID, grant.Role.Name).Return([]*dbmodel.Role{{Base: dbmodel.Base{ID: roleID}}}, nil).Once() + grantDbMock.On("GetGrants", tenantID, roleID, grant.Object.Name, grant.ObjectName).Return(grants, nil).Once() + grantIDDbMock.On("GetGrantIDs", tenantID, grantID, "", false, true).Return([]*dbmodel.GrantID{ + { + Privilege: "PrivilegeLoad", + Grantor: dbmodel.User{Username: "root"}, + }, + { + Privilege: "*", + Grantor: dbmodel.User{Username: "root"}, + }, + }, nil).Once() entites, err = mockCatalog.ListGrant(ctx, tenantID, grant) require.NoError(t, err) - require.Equal(t, 3, len(entites)) + require.Equal(t, 2, len(entites)) + require.Equal(t, "foo", entites[0].Role.Name) + require.Equal(t, "Collection", entites[1].Object.Name) + require.Equal(t, "col1", entites[1].ObjectName) + require.Equal(t, "root", entites[1].Grantor.User.Name) + require.Equal(t, "*", entites[1].Grantor.Privilege.Name) } func TestTableCatalog_ListGrant_GetRolesError(t *testing.T) { @@ -1638,12 +1744,15 @@ func TestTableCatalog_ListGrant_GetGrantError(t *testing.T) { require.Error(t, err) } -func TestTableCatalog_ListGrant_DecodeError(t *testing.T) { +func TestTableCatalog_ListGrant_GetGrantIDError(t *testing.T) { var ( - grant *milvuspb.GrantEntity - grants []*dbmodel.Grant - err error + roleID int64 = 1 + grantID int64 = 10 + grant *milvuspb.GrantEntity + grants []*dbmodel.Grant + err error ) + grant = &milvuspb.GrantEntity{ Role: &milvuspb.RoleEntity{Name: "foo"}, Object: &milvuspb.ObjectEntity{Name: "Collection"}, @@ -1651,117 +1760,110 @@ func TestTableCatalog_ListGrant_DecodeError(t *testing.T) { } grants = []*dbmodel.Grant{ { + Base: dbmodel.Base{ID: grantID}, Role: dbmodel.Role{Name: "foo"}, Object: "Collection", ObjectName: "col1", - Detail: "decode error", }, } - roleDbMock.On("GetRoles", tenantID, grant.Role.Name).Return([]*dbmodel.Role{{Base: dbmodel.Base{ID: 1}}}, nil).Once() - grantDbMock.On("GetGrants", tenantID, int64(1), grant.Object.Name, grant.ObjectName).Return(grants, nil).Once() + roleDbMock.On("GetRoles", tenantID, grant.Role.Name).Return([]*dbmodel.Role{{Base: dbmodel.Base{ID: roleID}}}, nil).Once() + grantDbMock.On("GetGrants", tenantID, roleID, grant.Object.Name, grant.ObjectName).Return(grants, nil).Once() + grantIDDbMock.On("GetGrantIDs", tenantID, grantID, "", false, true).Return(nil, errors.New("test error")).Once() _, err = mockCatalog.ListGrant(ctx, tenantID, grant) require.Error(t, err) } -func TestTableCatalog_ListGrant_DetailLenError(t *testing.T) { +func TestTableCatalog_ListGrant_NotExistError(t *testing.T) { var ( - grant *milvuspb.GrantEntity - grants []*dbmodel.Grant - err error + grant *milvuspb.GrantEntity + err error ) grant = &milvuspb.GrantEntity{ Role: &milvuspb.RoleEntity{Name: "foo"}, Object: &milvuspb.ObjectEntity{Name: "Collection"}, ObjectName: "col1", } - grants = []*dbmodel.Grant{ - { - Role: dbmodel.Role{Name: "foo"}, - Object: "Collection", - ObjectName: "col1", - Detail: "[[\"admin\"]]", - }, - } roleDbMock.On("GetRoles", tenantID, grant.Role.Name).Return([]*dbmodel.Role{{Base: dbmodel.Base{ID: 1}}}, nil).Once() - grantDbMock.On("GetGrants", tenantID, int64(1), grant.Object.Name, grant.ObjectName).Return(grants, nil).Once() + grantDbMock.On("GetGrants", tenantID, int64(1), grant.Object.Name, grant.ObjectName).Return(nil, nil).Once() _, err = mockCatalog.ListGrant(ctx, tenantID, grant) require.Error(t, err) + require.True(t, common.IsKeyNotExistError(err)) +} + +func TestTableCatalog_DropGrant(t *testing.T) { + var ( + roleName = "foo" + roleID int64 = 10 + err error + ) + roleDbMock.On("GetRoles", tenantID, roleName).Return([]*dbmodel.Role{{Base: dbmodel.Base{ID: roleID}}}, nil).Once() + grantDbMock.On("Delete", tenantID, roleID, "", "").Return(nil).Once() + err = mockCatalog.DeleteGrant(ctx, tenantID, &milvuspb.RoleEntity{Name: roleName}) + require.NoError(t, err) + + roleDbMock.On("GetRoles", tenantID, roleName).Return(nil, errors.New("test error")).Once() + err = mockCatalog.DeleteGrant(ctx, tenantID, &milvuspb.RoleEntity{Name: roleName}) + require.Error(t, err) + + roleDbMock.On("GetRoles", tenantID, roleName).Return([]*dbmodel.Role{{Base: dbmodel.Base{ID: roleID}}}, nil).Once() + grantDbMock.On("Delete", tenantID, roleID, "", "").Return(errors.New("test error")).Once() + err = mockCatalog.DeleteGrant(ctx, tenantID, &milvuspb.RoleEntity{Name: roleName}) + require.Error(t, err) } func TestTableCatalog_ListPolicy(t *testing.T) { var ( - grant *milvuspb.GrantEntity - grants []*dbmodel.Grant - policies []string - err error + roleName1 = "foo1" + roleName2 = "foo1" + grantID1 int64 = 10 + grantID2 int64 = 100 + object1 = "obj1" + object2 = "obj2" + objectName1 = "col1" + objectName2 = "col2" + privilege1 = "PrivilegeInsert" + privilege2 = "PrivilegeQuery" + grants []*dbmodel.Grant + policies []string + err error ) - grant = &milvuspb.GrantEntity{ - Role: &milvuspb.RoleEntity{Name: "foo"}, - Object: &milvuspb.ObjectEntity{Name: "Collection"}, - ObjectName: "col1", - } grants = []*dbmodel.Grant{ { - Role: dbmodel.Role{Name: "foo"}, - Object: "Collection", - ObjectName: "col1", - Detail: "[[\"admin\",\"PrivilegeIndexDetail\"]]", + Base: dbmodel.Base{ID: grantID1}, + Role: dbmodel.Role{Name: roleName1}, + Object: object1, + ObjectName: objectName1, + }, + { + Base: dbmodel.Base{ID: grantID2}, + Role: dbmodel.Role{Name: roleName2}, + Object: object2, + ObjectName: objectName2, }, } grantDbMock.On("GetGrants", tenantID, int64(0), "", "").Return(grants, nil).Once() + grantIDDbMock.On("GetGrantIDs", tenantID, grantID1, "", false, false).Return([]*dbmodel.GrantID{ + {Privilege: privilege1}, + {Privilege: privilege2}, + }, nil).Once() + grantIDDbMock.On("GetGrantIDs", tenantID, grantID2, "", false, false).Return([]*dbmodel.GrantID{ + {Privilege: privilege1}, + }, nil).Once() policies, err = mockCatalog.ListPolicy(ctx, tenantID) require.NoError(t, err) - require.Equal(t, 1, len(policies)) - require.Equal(t, funcutil.PolicyForPrivilege(grant.Role.Name, grant.Object.Name, grant.ObjectName, "PrivilegeIndexDetail"), policies[0]) -} + require.Equal(t, 3, len(policies)) + require.Equal(t, funcutil.PolicyForPrivilege(roleName1, object1, objectName1, privilege1), policies[0]) -func TestTableCatalog_ListPolicy_GetGrantsError(t *testing.T) { grantDbMock.On("GetGrants", tenantID, int64(0), "", "").Return(nil, errors.New("test error")).Once() - - _, err := mockCatalog.ListPolicy(ctx, tenantID) - require.Error(t, err) -} - -func TestTableCatalog_ListPolicy_DetailLenError(t *testing.T) { - var ( - grants []*dbmodel.Grant - err error - ) - - grants = []*dbmodel.Grant{ - { - Role: dbmodel.Role{Name: "foo"}, - Object: "Collection", - ObjectName: "col1", - Detail: "decode error", - }, - } - grantDbMock.On("GetGrants", tenantID, int64(0), "", "").Return(grants, nil).Once() - _, err = mockCatalog.ListPolicy(ctx, tenantID) require.Error(t, err) -} -func TestTableCatalog_ListPolicy_DecodeError(t *testing.T) { - var ( - grants []*dbmodel.Grant - err error - ) - - grants = []*dbmodel.Grant{ - { - Role: dbmodel.Role{Name: "foo"}, - Object: "Collection", - ObjectName: "col1", - Detail: "[[\"admin\"]]", - }, - } grantDbMock.On("GetGrants", tenantID, int64(0), "", "").Return(grants, nil).Once() - + grantIDDbMock.On("GetGrantIDs", tenantID, grantID1, "", false, false).Return(nil, errors.New("test error")).Once() _, err = mockCatalog.ListPolicy(ctx, tenantID) require.Error(t, err) } diff --git a/internal/metastore/kv/rootcoord/kv_catalog.go b/internal/metastore/kv/rootcoord/kv_catalog.go index 6812eb9150..a00fd138ba 100644 --- a/internal/metastore/kv/rootcoord/kv_catalog.go +++ b/internal/metastore/kv/rootcoord/kv_catalog.go @@ -5,6 +5,10 @@ import ( "encoding/json" "fmt" + "github.com/milvus-io/milvus/internal/util/crypto" + + "github.com/milvus-io/milvus/internal/util" + "github.com/golang/protobuf/proto" "go.uber.org/zap" @@ -16,7 +20,6 @@ import ( "github.com/milvus-io/milvus/internal/proto/internalpb" "github.com/milvus-io/milvus/internal/proto/milvuspb" "github.com/milvus-io/milvus/internal/proto/schemapb" - "github.com/milvus-io/milvus/internal/util" "github.com/milvus-io/milvus/internal/util/funcutil" "github.com/milvus-io/milvus/internal/util/typeutil" ) @@ -627,7 +630,7 @@ func (kc *Catalog) ListRole(ctx context.Context, tenant string, entity *milvuspb for _, key := range keys { roleMappingInfos := typeutil.AfterN(key, roleMappingKey+"/", "/") if len(roleMappingInfos) != 2 { - log.Warn("invalid role mapping key", zap.String("key", key)) + log.Warn("invalid role mapping key", zap.String("string", key), zap.String("sub_string", roleMappingKey)) continue } username := roleMappingInfos[0] @@ -657,7 +660,7 @@ func (kc *Catalog) ListRole(ctx context.Context, tenant string, entity *milvuspb for _, key := range keys { infoArr := typeutil.AfterN(key, roleKey+"/", "/") if len(infoArr) != 1 || len(infoArr[0]) == 0 { - log.Warn("invalid role key", zap.String("key", key)) + log.Warn("invalid role key", zap.String("string", key), zap.String("sub_string", roleKey)) continue } appendRoleResult(infoArr[0]) @@ -689,7 +692,7 @@ func (kc *Catalog) getRolesByUsername(tenant string, username string) ([]string, for _, key := range keys { roleMappingInfos := typeutil.AfterN(key, k+"/", "/") if len(roleMappingInfos) != 1 { - log.Warn("invalid role mapping key", zap.String("key", key)) + log.Warn("invalid role mapping key", zap.String("string", key), zap.String("sub_string", k)) continue } roles = append(roles, roleMappingInfos[0]) @@ -757,13 +760,17 @@ func (kc *Catalog) ListUser(ctx context.Context, tenant string, entity *milvuspb } func (kc *Catalog) AlterGrant(ctx context.Context, tenant string, entity *milvuspb.GrantEntity, operateType milvuspb.OperatePrivilegeType) error { - privilegeName := entity.Grantor.Privilege.Name - k := funcutil.HandleTenantForEtcdKey(GranteePrefix, tenant, fmt.Sprintf("%s/%s/%s", entity.Role.Name, entity.Object.Name, entity.ObjectName)) + var ( + privilegeName = entity.Grantor.Privilege.Name + k = funcutil.HandleTenantForEtcdKey(GranteePrefix, tenant, fmt.Sprintf("%s/%s/%s", entity.Role.Name, entity.Object.Name, entity.ObjectName)) + idStr string + v string + err error + ) - curGrantPrivilegeEntity := &milvuspb.GrantPrivilegeEntity{} - v, err := kc.Txn.Load(k) + v, err = kc.Txn.Load(k) if err != nil { - log.Warn("fail to load grant privilege entity", zap.String("key", k), zap.Any("type", operateType), zap.Error(err)) + log.Error("fail to load grant privilege entity", zap.String("key", k), zap.Any("type", operateType), zap.Error(err)) if funcutil.IsRevoke(operateType) { if common.IsKeyNotExistError(err) { return common.NewIgnorableError(fmt.Errorf("the grant[%s] isn't existed", k)) @@ -773,85 +780,71 @@ func (kc *Catalog) AlterGrant(ctx context.Context, tenant string, entity *milvus if !common.IsKeyNotExistError(err) { return err } - curGrantPrivilegeEntity.Entities = append(curGrantPrivilegeEntity.Entities, &milvuspb.GrantorEntity{ - Privilege: &milvuspb.PrivilegeEntity{Name: privilegeName}, - User: &milvuspb.UserEntity{Name: entity.Grantor.User.Name}, - }) - } else { - err = proto.Unmarshal([]byte(v), curGrantPrivilegeEntity) + + idStr = crypto.MD5(k) + err = kc.Txn.Save(k, idStr) if err != nil { - log.Error("fail to unmarshal the grant privilege entity", zap.String("key", k), zap.Any("type", operateType), zap.Error(err)) + log.Error("fail to allocate id when altering the grant", zap.Error(err)) return err } - isExisted := false - dropIndex := -1 - - for entityIndex, grantorEntity := range curGrantPrivilegeEntity.Entities { - if grantorEntity.Privilege.Name == privilegeName { - isExisted = true - dropIndex = entityIndex - break - } - } - if !isExisted && funcutil.IsGrant(operateType) { - curGrantPrivilegeEntity.Entities = append(curGrantPrivilegeEntity.Entities, &milvuspb.GrantorEntity{ - Privilege: &milvuspb.PrivilegeEntity{Name: privilegeName}, - User: &milvuspb.UserEntity{Name: entity.Grantor.User.Name}, - }) - } else if isExisted && funcutil.IsGrant(operateType) { - return common.NewIgnorableError(fmt.Errorf("the privilege[%s] is granted", privilegeName)) - } else if !isExisted && funcutil.IsRevoke(operateType) { - return common.NewIgnorableError(fmt.Errorf("the privilege[%s] isn't granted", privilegeName)) - } else if isExisted && funcutil.IsRevoke(operateType) { - curGrantPrivilegeEntity.Entities = append(curGrantPrivilegeEntity.Entities[:dropIndex], curGrantPrivilegeEntity.Entities[dropIndex+1:]...) - } + } else { + idStr = v } - - if funcutil.IsRevoke(operateType) && len(curGrantPrivilegeEntity.Entities) == 0 { - err = kc.Txn.Remove(k) - if err != nil { - log.Error("fail to remove the grant privilege entity", zap.String("key", k), zap.Error(err)) + k = funcutil.HandleTenantForEtcdKey(GranteeIDPrefix, tenant, fmt.Sprintf("%s/%s", idStr, privilegeName)) + _, err = kc.Txn.Load(k) + if err != nil { + log.Error("fail to load the grantee id", zap.String("key", k), zap.Error(err)) + if !common.IsKeyNotExistError(err) { + return err + } + if funcutil.IsRevoke(operateType) { + return common.NewIgnorableError(fmt.Errorf("the grantee-id[%s] isn't existed", k)) + } + if funcutil.IsGrant(operateType) { + if err = kc.Txn.Save(k, entity.Grantor.User.Name); err != nil { + log.Error("fail to save the grantee id", zap.String("key", k), zap.Error(err)) + } return err } return nil } - - saveValue, err := proto.Marshal(curGrantPrivilegeEntity) - if err != nil { - log.Error("fail to marshal the grant privilege entity", zap.String("key", k), zap.Any("type", operateType), zap.Error(err)) - return fmt.Errorf("fail to marshal grant info, key:%s, err:%w", k, err) - } - err = kc.Txn.Save(k, string(saveValue)) - if err != nil { - log.Error("fail to save the grant privilege entity", zap.String("key", k), zap.Any("type", operateType), zap.Error(err)) + if funcutil.IsRevoke(operateType) { + if err = kc.Txn.Remove(k); err != nil { + log.Error("fail to remove the grantee id", zap.String("key", k), zap.Error(err)) + return err + } return err } - return nil + return common.NewIgnorableError(fmt.Errorf("the privilege[%s] has been granted", privilegeName)) } func (kc *Catalog) ListGrant(ctx context.Context, tenant string, entity *milvuspb.GrantEntity) ([]*milvuspb.GrantEntity, error) { var entities []*milvuspb.GrantEntity - var k string + var granteeKey string appendGrantEntity := func(v string, object string, objectName string) error { - grantPrivilegeEntity := &milvuspb.GrantPrivilegeEntity{} - err := proto.Unmarshal([]byte(v), grantPrivilegeEntity) + granteeIDKey := funcutil.HandleTenantForEtcdKey(GranteeIDPrefix, tenant, v) + keys, values, err := kc.Txn.LoadWithPrefix(granteeIDKey) if err != nil { - log.Error("fail to unmarshal the grant privilege entity", zap.String("key", k), zap.Error(err)) + log.Error("fail to load the grantee ids", zap.String("key", granteeIDKey), zap.Error(err)) return err } - for _, grantorEntity := range grantPrivilegeEntity.Entities { - privilegeName := util.PrivilegeNameForAPI(grantorEntity.Privilege.Name) - if grantorEntity.Privilege.Name == util.AnyWord { + for i, key := range keys { + granteeIDInfos := typeutil.AfterN(key, granteeIDKey+"/", "/") + if len(granteeIDInfos) != 1 { + log.Warn("invalid grantee id", zap.String("string", key), zap.String("sub_string", granteeIDKey)) + continue + } + privilegeName := util.PrivilegeNameForAPI(granteeIDInfos[0]) + if granteeIDInfos[0] == util.AnyWord { privilegeName = util.AnyWord } - entities = append(entities, &milvuspb.GrantEntity{ Role: &milvuspb.RoleEntity{Name: entity.Role.Name}, Object: &milvuspb.ObjectEntity{Name: object}, ObjectName: objectName, Grantor: &milvuspb.GrantorEntity{ - User: &milvuspb.UserEntity{Name: grantorEntity.User.Name}, + User: &milvuspb.UserEntity{Name: values[i]}, Privilege: &milvuspb.PrivilegeEntity{Name: privilegeName}, }, }) @@ -860,10 +853,10 @@ func (kc *Catalog) ListGrant(ctx context.Context, tenant string, entity *milvusp } if !funcutil.IsEmptyString(entity.ObjectName) && entity.Object != nil && !funcutil.IsEmptyString(entity.Object.Name) { - k = funcutil.HandleTenantForEtcdKey(GranteePrefix, tenant, fmt.Sprintf("%s/%s/%s", entity.Role.Name, entity.Object.Name, entity.ObjectName)) - v, err := kc.Txn.Load(k) + granteeKey = funcutil.HandleTenantForEtcdKey(GranteePrefix, tenant, fmt.Sprintf("%s/%s/%s", entity.Role.Name, entity.Object.Name, entity.ObjectName)) + v, err := kc.Txn.Load(granteeKey) if err != nil { - log.Error("fail to load the grant privilege entity", zap.String("key", k), zap.Error(err)) + log.Error("fail to load the grant privilege entity", zap.String("key", granteeKey), zap.Error(err)) return entities, err } err = appendGrantEntity(v, entity.Object.Name, entity.ObjectName) @@ -871,16 +864,16 @@ func (kc *Catalog) ListGrant(ctx context.Context, tenant string, entity *milvusp return entities, err } } else { - k = funcutil.HandleTenantForEtcdKey(GranteePrefix, tenant, entity.Role.Name) - keys, values, err := kc.Txn.LoadWithPrefix(k) + granteeKey = funcutil.HandleTenantForEtcdKey(GranteePrefix, tenant, entity.Role.Name) + keys, values, err := kc.Txn.LoadWithPrefix(granteeKey) if err != nil { - log.Error("fail to load grant privilege entities", zap.String("key", k), zap.Error(err)) + log.Error("fail to load grant privilege entities", zap.String("key", granteeKey), zap.Error(err)) return entities, err } for i, key := range keys { - grantInfos := typeutil.AfterN(key, k+"/", "/") + grantInfos := typeutil.AfterN(key, granteeKey+"/", "/") if len(grantInfos) != 2 { - log.Warn("invalid grant key", zap.String("key", key)) + log.Warn("invalid grantee key", zap.String("string", key), zap.String("sub_string", granteeKey)) continue } err = appendGrantEntity(values[i], grantInfos[0], grantInfos[1]) @@ -893,30 +886,47 @@ func (kc *Catalog) ListGrant(ctx context.Context, tenant string, entity *milvusp return entities, nil } +func (kc *Catalog) DeleteGrant(ctx context.Context, tenant string, role *milvuspb.RoleEntity) error { + var ( + k = funcutil.HandleTenantForEtcdKey(GranteePrefix, tenant, role.Name+"/") + err error + ) + + if err = kc.Txn.RemoveWithPrefix(k); err != nil { + log.Error("fail to remove with the prefix", zap.String("key", k), zap.Error(err)) + } + return err +} + func (kc *Catalog) ListPolicy(ctx context.Context, tenant string) ([]string, error) { var grantInfoStrs []string - k := funcutil.HandleTenantForEtcdKey(GranteePrefix, tenant, "") - keys, values, err := kc.Txn.LoadWithPrefix(k) + granteeKey := funcutil.HandleTenantForEtcdKey(GranteePrefix, tenant, "") + keys, values, err := kc.Txn.LoadWithPrefix(granteeKey) if err != nil { - log.Error("fail to load all grant privilege entities", zap.String("key", k), zap.Error(err)) + log.Error("fail to load all grant privilege entities", zap.String("key", granteeKey), zap.Error(err)) return []string{}, err } for i, key := range keys { - grantInfos := typeutil.AfterN(key, k+"/", "/") + grantInfos := typeutil.AfterN(key, granteeKey+"/", "/") if len(grantInfos) != 3 { - log.Warn("invalid grant key", zap.String("key", key)) + log.Warn("invalid grantee key", zap.String("string", key), zap.String("sub_string", granteeKey)) continue } - grantPrivilegeEntity := &milvuspb.GrantPrivilegeEntity{} - err = proto.Unmarshal([]byte(values[i]), grantPrivilegeEntity) + granteeIDKey := funcutil.HandleTenantForEtcdKey(GranteeIDPrefix, tenant, values[i]) + idKeys, _, err := kc.Txn.LoadWithPrefix(granteeIDKey) if err != nil { - log.Warn("fail to unmarshal the grant privilege entity", zap.String("key", key), zap.Error(err)) - continue + log.Error("fail to load the grantee ids", zap.String("key", granteeIDKey), zap.Error(err)) + return []string{}, err } - for _, grantorInfo := range grantPrivilegeEntity.Entities { + for _, idKey := range idKeys { + granteeIDInfos := typeutil.AfterN(idKey, granteeIDKey+"/", "/") + if len(granteeIDInfos) != 1 { + log.Warn("invalid grantee id", zap.String("string", idKey), zap.String("sub_string", granteeIDKey)) + continue + } grantInfoStrs = append(grantInfoStrs, - funcutil.PolicyForPrivilege(grantInfos[0], grantInfos[1], grantInfos[2], grantorInfo.Privilege.Name)) + funcutil.PolicyForPrivilege(grantInfos[0], grantInfos[1], grantInfos[2], granteeIDInfos[0])) } } return grantInfoStrs, nil @@ -934,7 +944,7 @@ func (kc *Catalog) ListUserRole(ctx context.Context, tenant string) ([]string, e for _, key := range keys { userRolesInfos := typeutil.AfterN(key, k+"/", "/") if len(userRolesInfos) != 2 { - log.Warn("invalid user-role key", zap.String("key", key)) + log.Warn("invalid user-role key", zap.String("string", key), zap.String("sub_string", k)) continue } userRoles = append(userRoles, funcutil.EncodeUserRoleCache(userRolesInfos[0], userRolesInfos[1])) diff --git a/internal/metastore/kv/rootcoord/rootcoord_constant.go b/internal/metastore/kv/rootcoord/rootcoord_constant.go index c3579d1542..d01f86c02a 100644 --- a/internal/metastore/kv/rootcoord/rootcoord_constant.go +++ b/internal/metastore/kv/rootcoord/rootcoord_constant.go @@ -30,6 +30,9 @@ const ( // RoleMappingPrefix prefix for mapping between user and role RoleMappingPrefix = ComponentPrefix + CommonCredentialPrefix + "/user-role-mapping" - // GranteePrefix prefix for mapping among user or role, resource type, resource name + // GranteePrefix prefix for mapping among role, resource type, resource name GranteePrefix = ComponentPrefix + CommonCredentialPrefix + "/grantee-privileges" + + // GranteeIDPrefix prefix for mapping among privilege and grantor + GranteeIDPrefix = ComponentPrefix + CommonCredentialPrefix + "/grantee-id" ) diff --git a/internal/proxy/meta_cache.go b/internal/proxy/meta_cache.go index 935692e50e..e3a13dca1e 100644 --- a/internal/proxy/meta_cache.go +++ b/internal/proxy/meta_cache.go @@ -682,15 +682,15 @@ func (m *MetaCache) InitPolicyInfo(info []string, userRoles []string) { } func (m *MetaCache) GetPrivilegeInfo(ctx context.Context) []string { - m.mu.Lock() - defer m.mu.Unlock() + m.mu.RLock() + defer m.mu.RUnlock() return util.StringList(m.privilegeInfos) } func (m *MetaCache) GetUserRole(user string) []string { - m.mu.Lock() - defer m.mu.Unlock() + m.mu.RLock() + defer m.mu.RUnlock() return util.StringList(m.userToRoles[user]) } diff --git a/internal/rootcoord/meta_table.go b/internal/rootcoord/meta_table.go index 0d8d00791e..fcdde27fc8 100644 --- a/internal/rootcoord/meta_table.go +++ b/internal/rootcoord/meta_table.go @@ -882,7 +882,9 @@ func (mt *MetaTable) AddCredential(credInfo *internalpb.CredentialInfo) error { return err } if len(usernames) >= Params.ProxyCfg.MaxUserNum { - return errors.New("unable to add user because the number of users has reached the limit") + errMsg := "unable to add user because the number of users has reached the limit" + log.Error(errMsg, zap.Int("max_user_num", Params.ProxyCfg.MaxUserNum)) + return errors.New(errMsg) } if origin, _ := mt.catalog.GetCredential(mt.ctx, credInfo.Username); origin != nil { @@ -902,6 +904,9 @@ func (mt *MetaTable) AlterCredential(credInfo *internalpb.CredentialInfo) error return fmt.Errorf("username is empty") } + mt.permissionLock.Lock() + defer mt.permissionLock.Unlock() + credential := &model.Credential{ Username: credInfo.Username, EncryptedPassword: credInfo.EncryptedPassword, @@ -911,6 +916,9 @@ func (mt *MetaTable) AlterCredential(credInfo *internalpb.CredentialInfo) error // GetCredential get credential by username func (mt *MetaTable) GetCredential(username string) (*internalpb.CredentialInfo, error) { + mt.permissionLock.RLock() + defer mt.permissionLock.RUnlock() + credential, err := mt.catalog.GetCredential(mt.ctx, username) return model.MarshalCredentialModel(credential), err } @@ -949,7 +957,9 @@ func (mt *MetaTable) CreateRole(tenant string, entity *milvuspb.RoleEntity) erro return err } if len(results) >= Params.ProxyCfg.MaxRoleNum { - return errors.New("unable to add role because the number of roles has reached the limit") + errMsg := "unable to add role because the number of roles has reached the limit" + log.Error(errMsg, zap.Int("max_role_num", Params.ProxyCfg.MaxRoleNum)) + return errors.New(errMsg) } return mt.catalog.CreateRole(mt.ctx, tenant, entity) @@ -1043,6 +1053,16 @@ func (mt *MetaTable) SelectGrant(tenant string, entity *milvuspb.GrantEntity) ([ return mt.catalog.ListGrant(mt.ctx, tenant, entity) } +func (mt *MetaTable) DropGrant(tenant string, role *milvuspb.RoleEntity) error { + if role == nil || funcutil.IsEmptyString(role.Name) { + return fmt.Errorf("the role entity is invalid when dropping the grant") + } + mt.permissionLock.Lock() + defer mt.permissionLock.Unlock() + + return mt.catalog.DeleteGrant(mt.ctx, tenant, role) +} + func (mt *MetaTable) ListPolicy(tenant string) ([]string, error) { mt.permissionLock.RLock() defer mt.permissionLock.RUnlock() diff --git a/internal/rootcoord/meta_table_test.go b/internal/rootcoord/meta_table_test.go index 421869f2a1..e2a20740c8 100644 --- a/internal/rootcoord/meta_table_test.go +++ b/internal/rootcoord/meta_table_test.go @@ -27,7 +27,6 @@ import ( "testing" "time" - "github.com/golang/protobuf/proto" "github.com/milvus-io/milvus/internal/common" "github.com/milvus-io/milvus/internal/kv" etcdkv "github.com/milvus-io/milvus/internal/kv/etcd" @@ -86,6 +85,7 @@ type mockTestTxnKV struct { remove func(key string) error multiRemove func(keys []string) error load func(key string) (string, error) + removeWithPrefix func(key string) error } func (m *mockTestTxnKV) LoadWithPrefix(key string) ([]string, []string, error) { @@ -116,6 +116,10 @@ func (m *mockTestTxnKV) Load(key string) (string, error) { return m.load(key) } +func (m *mockTestTxnKV) RemoveWithPrefix(key string) error { + return m.removeWithPrefix(key) +} + func generateMetaTable(t *testing.T) (*MetaTable, *mockTestKV, *mockTestTxnKV, func()) { rand.Seed(time.Now().UnixNano()) randVal := rand.Int() @@ -937,70 +941,82 @@ func TestRbacOperatePrivilege(t *testing.T) { entity.Grantor.User = &milvuspb.UserEntity{Name: "user2"} err = mt.OperatePrivilege(util.DefaultTenant, entity, milvuspb.OperatePrivilegeType_Revoke) assert.NotNil(t, err) + err = mt.OperatePrivilege(util.DefaultTenant, entity, milvuspb.OperatePrivilegeType_Grant) assert.NotNil(t, err) + mockTxnKV.load = func(key string) (string, error) { - return "unmarshal", nil + return "", common.NewKeyNotExistError(key) + } + err = mt.OperatePrivilege(util.DefaultTenant, entity, milvuspb.OperatePrivilegeType_Revoke) + assert.NotNil(t, err) + assert.True(t, common.IsIgnorableError(err)) + + mockTxnKV.load = func(key string) (string, error) { + return "", common.NewKeyNotExistError(key) + } + err = mt.OperatePrivilege(util.DefaultTenant, entity, milvuspb.OperatePrivilegeType_Grant) + assert.Nil(t, err) + + granteeKey := funcutil.HandleTenantForEtcdKey(rootcoord.GranteePrefix, util.DefaultTenant, fmt.Sprintf("%s/%s/%s", entity.Role.Name, entity.Object.Name, entity.ObjectName)) + granteeID := "123456" + mockTxnKV.load = func(key string) (string, error) { + if key == granteeKey { + return granteeID, nil + } + return "", errors.New("test error") } err = mt.OperatePrivilege(util.DefaultTenant, entity, milvuspb.OperatePrivilegeType_Grant) assert.NotNil(t, err) mockTxnKV.load = func(key string) (string, error) { - return "fail", common.NewKeyNotExistError(key) + if key == granteeKey { + return granteeID, nil + } + return "", common.NewKeyNotExistError(key) + } + err = mt.OperatePrivilege(util.DefaultTenant, entity, milvuspb.OperatePrivilegeType_Revoke) + assert.NotNil(t, err) + assert.True(t, common.IsIgnorableError(err)) + + mockTxnKV.save = func(key, value string) error { + return errors.New("test error") + } + err = mt.OperatePrivilege(util.DefaultTenant, entity, milvuspb.OperatePrivilegeType_Grant) + assert.NotNil(t, err) + + mockTxnKV.save = func(key, value string) error { + return nil } err = mt.OperatePrivilege(util.DefaultTenant, entity, milvuspb.OperatePrivilegeType_Grant) assert.Nil(t, err) - grantPrivilegeEntity := &milvuspb.GrantPrivilegeEntity{Entities: []*milvuspb.GrantorEntity{ - {User: &milvuspb.UserEntity{Name: "aaa"}, Privilege: &milvuspb.PrivilegeEntity{Name: commonpb.ObjectPrivilege_PrivilegeLoad.String()}}, - }} mockTxnKV.load = func(key string) (string, error) { - grantPrivilegeEntityByte, _ := proto.Marshal(grantPrivilegeEntity) - return string(grantPrivilegeEntityByte), nil + if key == granteeKey { + return granteeID, nil + } + return "", nil } err = mt.OperatePrivilege(util.DefaultTenant, entity, milvuspb.OperatePrivilegeType_Grant) - assert.Equal(t, true, common.IsIgnorableError(err)) - entity.Grantor.Privilege = &milvuspb.PrivilegeEntity{Name: commonpb.ObjectPrivilege_PrivilegeRelease.String()} - err = mt.OperatePrivilege(util.DefaultTenant, entity, milvuspb.OperatePrivilegeType_Revoke) - assert.Equal(t, true, common.IsIgnorableError(err)) - grantPrivilegeEntity = &milvuspb.GrantPrivilegeEntity{Entities: []*milvuspb.GrantorEntity{ - {User: &milvuspb.UserEntity{Name: "user2"}, Privilege: &milvuspb.PrivilegeEntity{Name: commonpb.ObjectPrivilege_PrivilegeLoad.String()}}, - }} - mockTxnKV.load = func(key string) (string, error) { - grantPrivilegeEntityByte, _ := proto.Marshal(grantPrivilegeEntity) - return string(grantPrivilegeEntityByte), nil - } - err = mt.OperatePrivilege(util.DefaultTenant, entity, milvuspb.OperatePrivilegeType_Grant) - assert.Nil(t, err) - grantPrivilegeEntity = &milvuspb.GrantPrivilegeEntity{Entities: []*milvuspb.GrantorEntity{ - {User: &milvuspb.UserEntity{Name: "user2"}, Privilege: &milvuspb.PrivilegeEntity{Name: commonpb.ObjectPrivilege_PrivilegeRelease.String()}}, - }} - mockTxnKV.load = func(key string) (string, error) { - grantPrivilegeEntityByte, _ := proto.Marshal(grantPrivilegeEntity) - return string(grantPrivilegeEntityByte), nil - } - mockTxnKV.remove = func(key string) error { - return fmt.Errorf("remove error") - } - err = mt.OperatePrivilege(util.DefaultTenant, entity, milvuspb.OperatePrivilegeType_Revoke) assert.NotNil(t, err) + assert.True(t, common.IsIgnorableError(err)) + + mockTxnKV.load = func(key string) (string, error) { + if key == granteeKey { + return granteeID, nil + } + return "", nil + } mockTxnKV.remove = func(key string) error { return nil } err = mt.OperatePrivilege(util.DefaultTenant, entity, milvuspb.OperatePrivilegeType_Revoke) assert.Nil(t, err) - grantPrivilegeEntity = &milvuspb.GrantPrivilegeEntity{Entities: []*milvuspb.GrantorEntity{ - {User: &milvuspb.UserEntity{Name: "u3"}, Privilege: &milvuspb.PrivilegeEntity{Name: commonpb.ObjectPrivilege_PrivilegeInsert.String()}}, - }} - mockTxnKV.load = func(key string) (string, error) { - grantPrivilegeEntityByte, _ := proto.Marshal(grantPrivilegeEntity) - return string(grantPrivilegeEntityByte), nil + mockTxnKV.remove = func(key string) error { + return errors.New("test error") } - mockTxnKV.save = func(key, value string) error { - return fmt.Errorf("save error") - } - err = mt.OperatePrivilege(util.DefaultTenant, entity, milvuspb.OperatePrivilegeType_Grant) + err = mt.OperatePrivilege(util.DefaultTenant, entity, milvuspb.OperatePrivilegeType_Revoke) assert.NotNil(t, err) } @@ -1013,71 +1029,95 @@ func TestRbacSelectGrant(t *testing.T) { _, err = mt.SelectGrant(util.DefaultTenant, entity) assert.NotNil(t, err) - //entity.Role = &milvuspb.RoleEntity{Name: "admin"} + entity.Role = &milvuspb.RoleEntity{Name: ""} _, err = mt.SelectGrant(util.DefaultTenant, entity) assert.NotNil(t, err) + entity.Role = &milvuspb.RoleEntity{Name: "admin"} + entity.ObjectName = "Collection" + entity.Object = &milvuspb.ObjectEntity{Name: "col"} mockTxnKV.load = func(key string) (string, error) { - return "unmarshal error", nil + return "", errors.New("test error") } _, err = mt.SelectGrant(util.DefaultTenant, entity) assert.NotNil(t, err) + granteeID := "123456" + mockTxnKV.load = func(key string) (string, error) { + return granteeID, nil + } + mockTxnKV.loadWithPrefix = func(key string) ([]string, []string, error) { + return nil, nil, errors.New("test error") + } + _, err = mt.SelectGrant(util.DefaultTenant, entity) + assert.NotNil(t, err) + + mockTxnKV.loadWithPrefix = func(key string) ([]string, []string, error) { + return []string{key + "/PrivilegeInsert", key + "/*", key + "/a/b"}, []string{"root", "root", "root"}, nil + } + grantEntities, err := mt.SelectGrant(util.DefaultTenant, entity) + assert.Nil(t, err) + assert.Equal(t, 2, len(grantEntities)) + entity.Role = &milvuspb.RoleEntity{Name: "role1"} - entity.ObjectName = "col1" - entity.Object = &milvuspb.ObjectEntity{Name: "Collection"} - - grantPrivilegeEntity := &milvuspb.GrantPrivilegeEntity{Entities: []*milvuspb.GrantorEntity{ - {User: &milvuspb.UserEntity{Name: "aaa"}, Privilege: &milvuspb.PrivilegeEntity{Name: "111"}}, - {User: &milvuspb.UserEntity{Name: "bbb"}, Privilege: &milvuspb.PrivilegeEntity{Name: "222"}}, - {User: &milvuspb.UserEntity{Name: "ccc"}, Privilege: &milvuspb.PrivilegeEntity{Name: "*"}}, - }} - grantPrivilegeEntityByte, _ := proto.Marshal(grantPrivilegeEntity) - - mockTxnKV.load = func(key string) (string, error) { - return "unmarshal error", nil - } - _, err = mt.SelectGrant(util.DefaultTenant, entity) - assert.NotNil(t, err) - - mockTxnKV.load = func(key string) (string, error) { - return string(grantPrivilegeEntityByte), nil - } - results, err := mt.SelectGrant(util.DefaultTenant, entity) - assert.Nil(t, err) - assert.Equal(t, 3, len(results)) - - mockTxnKV.load = func(key string) (string, error) { - return "", fmt.Errorf("load error") - } - _, err = mt.SelectGrant(util.DefaultTenant, entity) - assert.NotNil(t, err) - - grantPrivilegeEntity2 := &milvuspb.GrantPrivilegeEntity{Entities: []*milvuspb.GrantorEntity{ - {User: &milvuspb.UserEntity{Name: "ccc"}, Privilege: &milvuspb.PrivilegeEntity{Name: "333"}}, - }} - grantPrivilegeEntityByte2, _ := proto.Marshal(grantPrivilegeEntity2) - - mockTxnKV.loadWithPrefix = func(key string) ([]string, []string, error) { - return []string{key + "/collection/col1", key + "/collection/col2", key + "/collection/a/col3"}, []string{string(grantPrivilegeEntityByte), string(grantPrivilegeEntityByte2), string(grantPrivilegeEntityByte2)}, nil - } entity.ObjectName = "" entity.Object = nil - results, err = mt.SelectGrant(util.DefaultTenant, entity) + mockTxnKV.loadWithPrefix = func(key string) ([]string, []string, error) { + return nil, nil, errors.New("test error") + } + _, err = mt.SelectGrant(util.DefaultTenant, entity) + assert.NotNil(t, err) + + granteeID1 := "123456" + granteeID2 := "147258" + mockTxnKV.loadWithPrefix = func(key string) ([]string, []string, error) { + if key == funcutil.HandleTenantForEtcdKey(rootcoord.GranteePrefix, util.DefaultTenant, entity.Role.Name) { + return []string{key + "/Collection/col1", key + "/Collection/col2", key + "/Collection/col1/x"}, []string{granteeID1, granteeID1, granteeID2}, nil + } + return nil, nil, errors.New("test error") + } + _, err = mt.SelectGrant(util.DefaultTenant, entity) + assert.NotNil(t, err) + + mockTxnKV.loadWithPrefix = func(key string) ([]string, []string, error) { + if key == funcutil.HandleTenantForEtcdKey(rootcoord.GranteePrefix, util.DefaultTenant, entity.Role.Name) { + return []string{key + "/Collection/col1", key + "/Collection/col2", key + "/Collection/col1/x"}, []string{granteeID1, granteeID1, granteeID2}, nil + } + return []string{key + "/PrivilegeInsert", key + "/*", key + "/a/b"}, []string{"root", "root", "root"}, nil + } + grantEntities, err = mt.SelectGrant(util.DefaultTenant, entity) assert.Nil(t, err) - assert.Equal(t, 4, len(results)) + assert.Equal(t, 4, len(grantEntities)) +} - mockTxnKV.loadWithPrefix = func(key string) ([]string, []string, error) { - return nil, nil, fmt.Errorf("load with prefix error") - } - _, err = mt.SelectGrant(util.DefaultTenant, entity) - assert.NotNil(t, err) +func TestRbacDropGrant(t *testing.T) { + mt, _, mockTxnKV, closeCli := generateMetaTable(t) + defer closeCli() + var ( + roleName = "foo" + entity *milvuspb.RoleEntity + err error + ) - mockTxnKV.loadWithPrefix = func(key string) ([]string, []string, error) { - return []string{key + "/collection/col1"}, []string{"unmarshal error"}, nil + err = mt.DropGrant(util.DefaultTenant, nil) + assert.Error(t, err) + + entity = &milvuspb.RoleEntity{Name: ""} + err = mt.DropGrant(util.DefaultTenant, entity) + assert.Error(t, err) + + entity.Name = roleName + mockTxnKV.removeWithPrefix = func(key string) error { + return nil } - _, err = mt.SelectGrant(util.DefaultTenant, entity) - assert.NotNil(t, err) + err = mt.DropGrant(util.DefaultTenant, entity) + assert.NoError(t, err) + + mockTxnKV.removeWithPrefix = func(key string) error { + return errors.New("test error") + } + err = mt.DropGrant(util.DefaultTenant, entity) + assert.Error(t, err) } func TestRbacListPolicy(t *testing.T) { @@ -1088,30 +1128,29 @@ func TestRbacListPolicy(t *testing.T) { return []string{}, []string{}, fmt.Errorf("load with prefix err") } policies, err := mt.ListPolicy(util.DefaultTenant) - assert.NotNil(t, err) + assert.Error(t, err) assert.Equal(t, 0, len(policies)) - grantPrivilegeEntity := &milvuspb.GrantPrivilegeEntity{Entities: []*milvuspb.GrantorEntity{ - {User: &milvuspb.UserEntity{Name: "aaa"}, Privilege: &milvuspb.PrivilegeEntity{Name: "111"}}, - {User: &milvuspb.UserEntity{Name: "bbb"}, Privilege: &milvuspb.PrivilegeEntity{Name: "222"}}, - }} - grantPrivilegeEntityByte, _ := proto.Marshal(grantPrivilegeEntity) - grantPrivilegeEntity2 := &milvuspb.GrantPrivilegeEntity{Entities: []*milvuspb.GrantorEntity{ - {User: &milvuspb.UserEntity{Name: "ccc"}, Privilege: &milvuspb.PrivilegeEntity{Name: "333"}}, - }} - grantPrivilegeEntityByte2, _ := proto.Marshal(grantPrivilegeEntity2) + granteeID1 := "123456" + granteeID2 := "147258" mockTxnKV.loadWithPrefix = func(key string) ([]string, []string, error) { - return []string{key + "/alice/collection/col1", key + "/tom/collection/col2", key + "/tom/collection/a/col2"}, []string{string(grantPrivilegeEntityByte), string(grantPrivilegeEntityByte2), string(grantPrivilegeEntityByte2)}, nil - } - policies, err = mt.ListPolicy(util.DefaultTenant) - assert.Nil(t, err) - assert.Equal(t, 3, len(policies)) - - mockTxnKV.loadWithPrefix = func(key string) ([]string, []string, error) { - return []string{key + "/alice/collection/col1"}, []string{"unmarshal error"}, nil + if key == funcutil.HandleTenantForEtcdKey(rootcoord.GranteePrefix, util.DefaultTenant, "") { + return []string{key + "/alice/collection/col1", key + "/tom/collection/col2", key + "/tom/collection/a/col2"}, []string{granteeID1, granteeID2, granteeID2}, nil + } + return []string{}, []string{}, errors.New("test error") } _, err = mt.ListPolicy(util.DefaultTenant) - assert.Nil(t, err) + assert.Error(t, err) + + mockTxnKV.loadWithPrefix = func(key string) ([]string, []string, error) { + if key == funcutil.HandleTenantForEtcdKey(rootcoord.GranteePrefix, util.DefaultTenant, "") { + return []string{key + "/alice/collection/col1", key + "/tom/collection/col2", key + "/tom/collection/a/col2"}, []string{granteeID1, granteeID2, granteeID2}, nil + } + return []string{key + "/PrivilegeInsert", key + "/*", key + "/a/b"}, []string{"root", "root", "root"}, nil + } + policies, err = mt.ListPolicy(util.DefaultTenant) + assert.NoError(t, err) + assert.Equal(t, 4, len(policies)) } func TestRbacListUserRole(t *testing.T) { diff --git a/internal/rootcoord/root_coord.go b/internal/rootcoord/root_coord.go index badaaeb0aa..ea498267e5 100644 --- a/internal/rootcoord/root_coord.go +++ b/internal/rootcoord/root_coord.go @@ -1061,7 +1061,7 @@ func (c *Core) initRbac() (initError error) { Object: &milvuspb.ObjectEntity{Name: commonpb.ObjectType_Global.String()}, ObjectName: util.AnyWord, Grantor: &milvuspb.GrantorEntity{ - User: &milvuspb.UserEntity{Name: util.RoleAdmin}, + User: &milvuspb.UserEntity{Name: util.UserRoot}, Privilege: &milvuspb.PrivilegeEntity{Name: globalPrivilege}, }, }, milvuspb.OperatePrivilegeType_Grant); initError != nil { @@ -1078,7 +1078,7 @@ func (c *Core) initRbac() (initError error) { Object: &milvuspb.ObjectEntity{Name: commonpb.ObjectType_Collection.String()}, ObjectName: util.AnyWord, Grantor: &milvuspb.GrantorEntity{ - User: &milvuspb.UserEntity{Name: util.RoleAdmin}, + User: &milvuspb.UserEntity{Name: util.UserRoot}, Privilege: &milvuspb.PrivilegeEntity{Name: collectionPrivilege}, }, }, milvuspb.OperatePrivilegeType_Grant); initError != nil { @@ -2540,6 +2540,11 @@ func (c *Core) DropRole(ctx context.Context, in *milvuspb.DropRoleRequest) (*com } } } + if err = c.MetaTable.DropGrant(util.DefaultTenant, &milvuspb.RoleEntity{Name: in.RoleName}); err != nil { + errMsg := "fail to drop the grant" + logger.Error(errMsg, zap.String("role_name", in.RoleName), zap.Error(err)) + return failStatus(commonpb.ErrorCode_DropRoleFailure, errMsg), err + } if err = c.MetaTable.DropRole(util.DefaultTenant, in.RoleName); err != nil { errMsg := "fail to drop the role" logger.Error(errMsg, zap.String("role_name", in.RoleName), zap.Error(err)) diff --git a/internal/util/crypto/crypto.go b/internal/util/crypto/crypto.go index cdbf032e6c..9b8e4d97ee 100644 --- a/internal/util/crypto/crypto.go +++ b/internal/util/crypto/crypto.go @@ -1,6 +1,7 @@ package crypto import ( + "crypto/md5" // #nosec "crypto/sha256" "encoding/base64" "encoding/hex" @@ -39,3 +40,9 @@ func Base64Decode(pwd string) (string, error) { func Base64Encode(pwd string) string { return base64.StdEncoding.EncodeToString([]byte(pwd)) } + +func MD5(str string) string { + // #nosec + data := md5.Sum([]byte(str)) + return hex.EncodeToString(data[:])[8:24] +} diff --git a/internal/util/crypto/crypto_test.go b/internal/util/crypto/crypto_test.go index 73e1bdbce2..4923680202 100644 --- a/internal/util/crypto/crypto_test.go +++ b/internal/util/crypto/crypto_test.go @@ -41,3 +41,7 @@ func TestBcryptCost(t *testing.T) { err = bcrypt.CompareHashAndPassword(bytes, []byte(correctPassword)) assert.NoError(t, err) } + +func TestMD5(t *testing.T) { + assert.Equal(t, "67f48520697662a2", MD5("These pretzels are making me thirsty.")) +} diff --git a/internal/util/typeutil/string_util_test.go b/internal/util/typeutil/string_util_test.go index 0bfa4da59f..a1c047827e 100644 --- a/internal/util/typeutil/string_util_test.go +++ b/internal/util/typeutil/string_util_test.go @@ -54,3 +54,8 @@ func TestAfter(t *testing.T) { res = After("abc", "ab") assert.Equal(t, res, "c") } + +func TestAfterN(t *testing.T) { + strs := AfterN("by-dev/meta/root-coord/credential/grantee-privileges/public/Global/*", "root-coord/credential/grantee-privileges/", "/") + assert.Len(t, strs, 3) +} diff --git a/scripts/sql/meta.sql b/scripts/sql/meta.sql index d90afdb710..365f25beda 100644 --- a/scripts/sql/meta.sql +++ b/scripts/sql/meta.sql @@ -219,7 +219,7 @@ CREATE TABLE if not exists milvus_meta.role ( is_deleted BOOL NOT NULL DEFAULT false, created_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, updated_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP on update current_timestamp, - INDEX idx_role_tenant_name (tenant_id, name), + INDEX idx_role_tenant_name (tenant_id, name, is_deleted), PRIMARY KEY (id) ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; @@ -232,7 +232,7 @@ CREATE TABLE if not exists milvus_meta.user_role ( is_deleted BOOL NOT NULL DEFAULT false, created_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, updated_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP on update current_timestamp, - INDEX idx_role_mapping_tenant_user_role (tenant_id, user_id, role_id), + INDEX idx_role_mapping_tenant_user_role (tenant_id, user_id, role_id, is_deleted), PRIMARY KEY (id) ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; @@ -243,10 +243,23 @@ CREATE TABLE if not exists milvus_meta.grant ( role_id BIGINT NOT NULL, object VARCHAR(128) NOT NULL, object_name VARCHAR(128) NOT NULL, - detail TEXT NOT NULL, is_deleted BOOL NOT NULL DEFAULT false, created_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, updated_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP on update current_timestamp, - INDEX idx_grant_principal_resource_tenant (tenant_id, role_id, object, object_name), + INDEX idx_grant_principal_resource_tenant (tenant_id, role_id, object, object_name, is_deleted), PRIMARY KEY (id) ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; + +-- grant-id +CREATE TABLE if not exists milvus_meta.grant_id ( + id BIGINT NOT NULL AUTO_INCREMENT, + tenant_id VARCHAR(128) DEFAULT NULL, + grant_id BIGINT NOT NULL, + grantor_id BIGINT NOT NULL, + privilege VARCHAR(128) NOT NULL, + is_deleted BOOL NOT NULL DEFAULT false, + created_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, + updated_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP on update current_timestamp, + INDEX idx_grant_id_tenant_grantor (tenant_id, grant_id, grantor_id, is_deleted), + PRIMARY KEY (id) +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; \ No newline at end of file