From 7552858897eeddd695acdc1ecca76ac426669191 Mon Sep 17 00:00:00 2001 From: Timmy Date: Fri, 9 Jun 2023 17:58:57 +0800 Subject: [PATCH 1/3] bugfix: update expression delete update sql (#252) --- .github/workflows/go.yml | 7 +++---- .gitignore | 1 + pkg/abac/pdp/evalctx/context.go | 5 +++-- pkg/abac/prp/policy.go | 4 ++-- pkg/api/model/handler/action_slz.go | 4 ++-- pkg/api/model/handler/query.go | 4 +++- pkg/database/dao/expression.go | 24 +++++++++++++++--------- pkg/database/dao/expression_test.go | 2 +- pkg/logging/formatter_json.go | 4 ++-- pkg/service/policy.go | 22 ++++++++++++++++++++-- pkg/task/handler/handler.go | 3 ++- 11 files changed, 54 insertions(+), 26 deletions(-) diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index b75fe194..cadc93e6 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -11,10 +11,10 @@ jobs: build: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v3 - name: Set up Go - uses: actions/setup-go@v2 + uses: actions/setup-go@v4 with: go-version: 1.18.1 @@ -25,7 +25,6 @@ jobs: run: make test - name: Lint - uses: golangci/golangci-lint-action@v2 + uses: golangci/golangci-lint-action@v3 with: version: v1.46.2 - skip-go-installation: true diff --git a/.gitignore b/.gitignore index 82b2864b..f601bb97 100644 --- a/.gitignore +++ b/.gitignore @@ -44,3 +44,4 @@ build.yml .md_configs.data .me_configs.data .codecc +.idea \ No newline at end of file diff --git a/pkg/abac/pdp/evalctx/context.go b/pkg/abac/pdp/evalctx/context.go index f8341300..6854a81c 100644 --- a/pkg/abac/pdp/evalctx/context.go +++ b/pkg/abac/pdp/evalctx/context.go @@ -205,8 +205,9 @@ func (c *EvalContext) InitEnvironments(cond condition.Condition, currentTime tim // GenTimeEnvsFromCache will return the same time-related envs if the tz and timestamp are same! // NOTE: cache only if the envs is same for every request -// if you will change the envs later(e.g. set some value from request, do not cache it!) -// at that time, you should remove this func, use a new collection like sync.Pool +// +// if you will change the envs later(e.g. set some value from request, do not cache it!) +// at that time, you should remove this func, use a new collection like sync.Pool func GenTimeEnvsFromCache(tz string, currentTime time.Time) (map[string]interface{}, error) { key := tz + strconv.FormatInt(currentTime.Unix(), 10) diff --git a/pkg/abac/prp/policy.go b/pkg/abac/prp/policy.go index 7c91d3ae..3560fa64 100644 --- a/pkg/abac/prp/policy.go +++ b/pkg/abac/prp/policy.go @@ -120,8 +120,8 @@ func NewPolicyManager() PolicyManager { // ListBySubjectAction 查询用于鉴权的policy列表 // policy有2个来源 -// 1. 普通权限(自定义权限, 继承的用户组权限) -// 2. 临时权限(只来自个人) +// 1. 普通权限(自定义权限, 继承的用户组权限) +// 2. 临时权限(只来自个人) func (m *policyManager) ListBySubjectAction( system string, subject types.Subject, diff --git a/pkg/api/model/handler/action_slz.go b/pkg/api/model/handler/action_slz.go index ce388d7f..3c314fa8 100644 --- a/pkg/api/model/handler/action_slz.go +++ b/pkg/api/model/handler/action_slz.go @@ -207,8 +207,8 @@ func validateRelatedResourceTypes(data []relatedResourceType, actionID string) ( } // validateActionAuthType will check the auth_type is valid or not -// 1. if len(data.RelatedResourceTypes) == 0, auth_type should be "abac" -// 2. if len(data.RelatedResourceTypes) > 0, auth_type should be "abac" OR "rbac" +// 1. if len(data.RelatedResourceTypes) == 0, auth_type should be "abac" +// 2. if len(data.RelatedResourceTypes) > 0, auth_type should be "abac" OR "rbac" // 2.1 if auth_type == "rbac", the related_resource_type[selection_mode] should be 'instance' func validateActionAuthType(authType string, relatedResourceTypes []relatedResourceType) (bool, string) { if authType == types.AuthTypeRBACStr { diff --git a/pkg/api/model/handler/query.go b/pkg/api/model/handler/query.go index 243532d9..3216be33 100644 --- a/pkg/api/model/handler/query.go +++ b/pkg/api/model/handler/query.go @@ -46,6 +46,7 @@ const ( // @Security AppCode // @Security AppSecret // @Router /api/v1/systems/{system_id}/query [get] +// //nolint:gocognit func SystemInfoQuery(c *gin.Context) { var query querySerializer @@ -65,8 +66,9 @@ func SystemInfoQuery(c *gin.Context) { BuildSystemInfoQueryResponse(c, systemID, fieldSet, false) } -//nolint:gocognit // BuildSystemInfoQueryResponse will only the data requested +// +//nolint:gocognit func BuildSystemInfoQueryResponse(c *gin.Context, systemID string, fieldSet *set.StringSet, isModelSharing bool) { // make the return data data := gin.H{} diff --git a/pkg/database/dao/expression.go b/pkg/database/dao/expression.go index 42ffe2b3..d3cd126f 100644 --- a/pkg/database/dao/expression.go +++ b/pkg/database/dao/expression.go @@ -203,11 +203,14 @@ func (m *expressionManager) updateUnreferencedExpressionType(fromType int64, toT func (m *expressionManager) updateReferencedExpressionTypeBeforeUpdateAt( fromType int64, toType int64, updatedAt int64, ) error { - sql := `UPDATE expression SET - type=? - WHERE type=? - AND updated_at < FROM_UNIXTIME(?) - AND pk IN (SELECT expression_pk FROM policy)` + sql := `UPDATE expression SET + type=? + WHERE pk IN (SELECT pk FROM + (SELECT pk FROM expression + WHERE type=? + AND updated_at < FROM_UNIXTIME(?) + AND pk IN (SELECT expression_pk FROM policy) + ) AS e)` return database.SqlxExec(m.DB, sql, toType, fromType, updatedAt) } @@ -215,9 +218,12 @@ func (m *expressionManager) deleteUnreferencedExpressionByTypeBeforeUpdateAt( _type int64, updatedAt int64, limit int64, ) (int64, error) { sql := `DELETE FROM expression - WHERE type=? - AND updated_at < FROM_UNIXTIME(?) - AND pk NOT IN (SELECT expression_pk FROM policy) - LIMIT ?` + WHERE pk IN (SELECT pk FROM + (SELECT pk FROM expression + WHERE type=? + AND updated_at < FROM_UNIXTIME(?) + AND pk NOT IN (SELECT expression_pk FROM policy) + LIMIT ? + ) AS e)` return database.SqlxDelete(m.DB, sql, _type, updatedAt, limit) } diff --git a/pkg/database/dao/expression_test.go b/pkg/database/dao/expression_test.go index 94b15dc6..502ed87d 100644 --- a/pkg/database/dao/expression_test.go +++ b/pkg/database/dao/expression_test.go @@ -198,7 +198,7 @@ func Test_expressionManager_ChangeReferencedExpressionTypeBeforeUpdateAt(t *test func Test_expressionManager_DeleteUnreferencedExpressionByTypeBeforeUpdateAt(t *testing.T) { database.RunWithMock(t, func(db *sqlx.DB, mock sqlmock.Sqlmock, t *testing.T) { mock.ExpectBegin() - mock.ExpectExec(`DELETE FROM expression WHERE type=`).WithArgs( + mock.ExpectExec(`DELETE FROM expression WHERE pk IN`).WithArgs( int64(-1), int64(0), int64(10000), ).WillReturnResult(sqlmock.NewResult(1, 1)) mock.ExpectCommit() diff --git a/pkg/logging/formatter_json.go b/pkg/logging/formatter_json.go index 253bdec5..f0225eb6 100644 --- a/pkg/logging/formatter_json.go +++ b/pkg/logging/formatter_json.go @@ -184,12 +184,12 @@ func (f *JSONFormatter) Format(entry *logrus.Entry) ([]byte, error) { // This is to not silently overwrite `time`, `msg`, `func` and `level` fields when // dumping it. If this code wasn't there doing: // -// logrus.WithField("level", 1).Info("hello") +// logrus.WithField("level", 1).Info("hello") // // Would just silently drop the user provided level. Instead with this code // it'll logged as: // -// {"level": "info", "fields.level": 1, "msg": "hello", "time": "..."} +// {"level": "info", "fields.level": 1, "msg": "hello", "time": "..."} // // It's not exported because it's still using Data in an opinionated way. It's to // avoid code duplication between the two default formatters. diff --git a/pkg/service/policy.go b/pkg/service/policy.go index 7fad65e5..2eb2354c 100644 --- a/pkg/service/policy.go +++ b/pkg/service/policy.go @@ -653,8 +653,8 @@ func (s *policyService) DeleteUnreferencedExpressions() error { // 2. 删除标记未被引用的expression // 由于删除时可能数量较大，耗时长，锁行数据较多，影响鉴权，所以需要循环删除，限制每次删除的记录数，以及最多执行删除多少次 - rowLimit := int64(5000) - maxAttempts := 100 // 相当于最多删除50万数据 + rowLimit := int64(10000) + maxAttempts := 100 // 相当于最多删除100万数据 for i := 0; i < maxAttempts; i++ { rowsAffected, err := s.expressionManger.DeleteUnreferencedExpressionByTypeBeforeUpdateAt( @@ -673,6 +673,24 @@ func (s *policyService) DeleteUnreferencedExpressions() error { } } + // 清理自定义权限的未被引用的expression + for i := 0; i < maxAttempts; i++ { + rowsAffected, err := s.expressionManger.DeleteUnreferencedExpressionByTypeBeforeUpdateAt( + expressionTypeCustom, + updateAt, + rowLimit, + ) + if err != nil { + return errorWrapf(err, "expressionManger.DeleteByTypeBeforeUpdateAt type=`%d`, updateAt=`%d`", + expressionTypeCustom, updateAt) + } + + // 如果已经没有需要删除的了，就停止 + if rowsAffected == 0 { + break + } + } + // 3. 标记未被引用的expression err = s.expressionManger.ChangeUnreferencedExpressionType(expressionTypeTemplate, expressionTypeUnreferenced) if err != nil { diff --git a/pkg/task/handler/handler.go b/pkg/task/handler/handler.go index 96274adb..aa51b7fa 100644 --- a/pkg/task/handler/handler.go +++ b/pkg/task/handler/handler.go @@ -161,7 +161,8 @@ func (h *groupAlterMessageHandler) alterSubjectActionGroupResource(subjectPK, ac groupPK, actionPK, ) - if err != nil { + // NOTE: action如果被删除, rbac_group_resource_policy中action_pks并没有清理, 这里可能出现操作查询不到的错误, 如果查询不到, 直接删除 + if err != nil && !errors.Is(err, sql.ErrNoRows) { return errorWrapf(err, "cacheimpls.GetGroupActionAuthorizedResource fail, groupPK=`%d`, actionPK=`%d`", groupPK, actionPK, From 25468fe8e220624052ec0b1789f3be234732c02a Mon Sep 17 00:00:00 2001 From: Timmy Date: Fri, 9 Jun 2023 18:11:27 +0800 Subject: [PATCH 2/3] v1.12.7 (#257) --- VERSION | 2 +- release.md | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/VERSION b/VERSION index 456e5c4a..41c8a73d 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.12.6 +1.12.7 diff --git a/release.md b/release.md index b65e9396..10fdc614 100644 --- a/release.md +++ b/release.md @@ -1,3 +1,8 @@ +# 1.12.7 + +- bugfix: delete unreferenced expression +- bugfix: rbac policy expression generate if action not found + # 1.12.6 - upgrade: /subjects-groups/belong api From c9a55ea2e9bcf01efc2ada40163a4b1a44ee661b Mon Sep 17 00:00:00 2001 From: Timmy Date: Mon, 17 Jul 2023 14:58:27 +0800 Subject: [PATCH 3/3] bugfix: subject system groups sql for mysql 8.0 (#258) --- pkg/database/dao/subject_system_group.go | 25 ++++++++++--------- pkg/database/dao/subject_system_group_test.go | 15 +++++------ 2 files changed, 21 insertions(+), 19 deletions(-) diff --git a/pkg/database/dao/subject_system_group.go b/pkg/database/dao/subject_system_group.go index e7578ec3..1b5bc13c 100644 --- a/pkg/database/dao/subject_system_group.go +++ b/pkg/database/dao/subject_system_group.go @@ -13,6 +13,7 @@ package dao import ( "database/sql" "errors" + "fmt" "github.com/jmoiron/sqlx" @@ -97,11 +98,11 @@ func (m *subjectSystemGroupManager) selectGroups( systemID string, subjectPKs []int64, ) error { - query := `SELECT + query := fmt.Sprintf(`SELECT subject_pk, - groups + %s FROM subject_system_group - WHERE system_id = ? AND subject_pk IN (?)` + WHERE system_id = ? AND subject_pk IN (?)`, "`groups`") return database.SqlxSelect(m.DB, groups, query, systemID, subjectPKs) } @@ -110,37 +111,37 @@ func (m *subjectSystemGroupManager) selectBySystemSubject( systemID string, subjectPK int64, ) error { - query := `SELECT + query := fmt.Sprintf(`SELECT pk, system_id, subject_pk, - groups, + %s, reversion FROM subject_system_group - WHERE system_id = ? AND subject_pk = ?` + WHERE system_id = ? AND subject_pk = ?`, "`groups`") return database.SqlxGet(m.DB, subjectSystemGroup, query, systemID, subjectPK) } func (m *subjectSystemGroupManager) insertWithTx(tx *sqlx.Tx, subjectSystemGroup *SubjectSystemGroup) error { - sql := `INSERT INTO subject_system_group ( + sql := fmt.Sprintf(`INSERT INTO subject_system_group ( system_id, subject_pk, - groups + %s ) VALUES ( :system_id, :subject_pk, :groups - )` + )`, "`groups`") return database.SqlxInsertWithTx(tx, sql, subjectSystemGroup) } func (m *subjectSystemGroupManager) updateWithTx(tx *sqlx.Tx, subjectSystemGroup *SubjectSystemGroup) (int64, error) { - sql := `UPDATE subject_system_group SET - groups = :groups, + sql := fmt.Sprintf(`UPDATE subject_system_group SET + %s = :groups, reversion = reversion + 1 WHERE system_id = :system_id AND subject_pk = :subject_pk - AND reversion = :reversion` + AND reversion = :reversion`, "`groups`") return database.SqlxUpdateWithTx(tx, sql, subjectSystemGroup) } diff --git a/pkg/database/dao/subject_system_group_test.go b/pkg/database/dao/subject_system_group_test.go index 70542d25..0f4bd79f 100644 --- a/pkg/database/dao/subject_system_group_test.go +++ b/pkg/database/dao/subject_system_group_test.go @@ -11,6 +11,7 @@ package dao import ( + "fmt" "testing" sqlmock "github.com/DATA-DOG/go-sqlmock" @@ -22,11 +23,11 @@ import ( func Test_subjectSystemGroupManager_ListGroups(t *testing.T) { database.RunWithMock(t, func(db *sqlx.DB, mock sqlmock.Sqlmock, t *testing.T) { - mockQuery := `^SELECT + mockQuery := fmt.Sprintf(`^SELECT subject_pk, - groups + %s FROM subject_system_group - WHERE system_id = (.*) AND subject_pk IN (.*)` + WHERE system_id = (.*) AND subject_pk IN (.*)`, "`groups`") mockRows := sqlmock.NewRows([]string{"subject_pk", "groups"}).AddRow(int64(1), "test") mock.ExpectQuery(mockQuery).WithArgs("system", int64(1)).WillReturnRows(mockRows) @@ -58,14 +59,14 @@ func Test_subjectSystemGroupManager_DeleteBySystemSubjectWithTx(t *testing.T) { func Test_subjectSystemGroupManager_GetBySystemSubject(t *testing.T) { database.RunWithMock(t, func(db *sqlx.DB, mock sqlmock.Sqlmock, t *testing.T) { - mockQuery := `^SELECT + mockQuery := fmt.Sprintf(`^SELECT pk, system_id, subject_pk, - groups, + %s, reversion FROM subject_system_group - WHERE system_id = (.*) AND subject_pk = (.*)` + WHERE system_id = (.*) AND subject_pk = (.*)`, "`groups`") mockRows := sqlmock.NewRows([]string{"system_id", "subject_pk", "groups", "reversion"}). AddRow("test", int64(1), "[]", int64(2)) mock.ExpectQuery(mockQuery).WithArgs("system", int64(1)).WillReturnRows(mockRows) @@ -108,7 +109,7 @@ func Test_subjectSystemGroupManager_CreateWithTx(t *testing.T) { func Test_subjectSystemGroupManager_UpdateWithTx(t *testing.T) { database.RunWithMock(t, func(db *sqlx.DB, mock sqlmock.Sqlmock, t *testing.T) { mock.ExpectBegin() - mock.ExpectExec(`^UPDATE subject_system_group SET groups = (.*)`).WithArgs( + mock.ExpectExec(fmt.Sprintf(`^UPDATE subject_system_group SET %s = (.*)`, "`groups`")).WithArgs( "[]", "system", int64(1), int64(2), ).WillReturnResult(sqlmock.NewResult(1, 1)) mock.ExpectCommit()