From a356dc75f297b651f7b1b0c541122253ba9b3a9f Mon Sep 17 00:00:00 2001 From: lance6716 Date: Wed, 30 Nov 2022 20:18:01 +0800 Subject: [PATCH 1/3] This is an automated cherry-pick of #7739 Signed-off-by: ti-chi-bot --- dm/pkg/checker/privilege.go | 34 +++++++- dm/pkg/checker/privilege_test.go | 141 +++++++++++++++++++++++++++++++ 2 files changed, 174 insertions(+), 1 deletion(-) diff --git a/dm/pkg/checker/privilege.go b/dm/pkg/checker/privilege.go index ecfa20333ca..19e1e50ebf3 100644 --- a/dm/pkg/checker/privilege.go +++ b/dm/pkg/checker/privilege.go @@ -25,6 +25,12 @@ import ( _ "github.com/pingcap/tidb/types/parser_driver" // for parser driver "github.com/pingcap/tidb/util/dbutil" "github.com/pingcap/tidb/util/filter" +<<<<<<< HEAD +======= + "github.com/pingcap/tidb/util/stringutil" + "github.com/pingcap/tiflow/dm/pkg/log" + "github.com/pingcap/tiflow/pkg/container/sortmap" +>>>>>>> a2d2a5213c (checker(dm): support wildcard in privilege checking (#7739)) "go.uber.org/zap" "github.com/pingcap/tiflow/dm/pkg/log" @@ -172,7 +178,7 @@ func verifyPrivileges(result *Result, grants []string, lackPriv map[mysql.Privil return NewError("grant has no user %s", grant) } - dbName := grantStmt.Level.DBName + dbPatChar, dbPatType := stringutil.CompilePattern(grantStmt.Level.DBName, '\\') tableName := grantStmt.Level.TableName switch grantStmt.Level.Level { case ast.GrantLevelGlobal: @@ -203,6 +209,7 @@ func verifyPrivileges(result *Result, grants []string, lackPriv map[mysql.Privil if priv == mysql.GrantPriv { continue } +<<<<<<< HEAD if _, ok := lackPriv[priv][dbName]; !ok { continue } @@ -210,26 +217,44 @@ func verifyPrivileges(result *Result, grants []string, lackPriv map[mysql.Privil if len(lackPriv[priv]) == 0 { delete(lackPriv, priv) } +======= + for dbName := range privs.dbs { + if stringutil.DoMatch(dbName, dbPatChar, dbPatType) { + delete(privs.dbs, dbName) + } + } +>>>>>>> a2d2a5213c (checker(dm): support wildcard in privilege checking (#7739)) } continue } if _, ok := lackPriv[privElem.Priv]; !ok { continue } +<<<<<<< HEAD if _, ok := lackPriv[privElem.Priv][dbName]; !ok { continue } +======= +>>>>>>> a2d2a5213c (checker(dm): support wildcard in privilege checking (#7739)) // dumpling could report error if an allow-list table is lack of privilege. // we only check that SELECT is granted on all columns, otherwise we can't SHOW CREATE TABLE if privElem.Priv == mysql.SelectPriv && len(privElem.Cols) != 0 { continue } +<<<<<<< HEAD delete(lackPriv[privElem.Priv], dbName) if len(lackPriv[privElem.Priv]) == 0 { delete(lackPriv, privElem.Priv) +======= + for dbName := range privs.dbs { + if stringutil.DoMatch(dbName, dbPatChar, dbPatType) { + delete(privs.dbs, dbName) + } +>>>>>>> a2d2a5213c (checker(dm): support wildcard in privilege checking (#7739)) } } case ast.GrantLevelTable: + dbName := grantStmt.Level.DBName for _, privElem := range grantStmt.Privs { // all privileges available at a given privilege level (except GRANT OPTION) // from https://dev.mysql.com/doc/refman/5.7/en/privileges-provided.html#priv_all @@ -241,6 +266,7 @@ func verifyPrivileges(result *Result, grants []string, lackPriv map[mysql.Privil if _, ok := lackPriv[priv][dbName]; !ok { continue } +<<<<<<< HEAD if _, ok := lackPriv[priv][dbName][tableName]; !ok { continue } @@ -251,6 +277,9 @@ func verifyPrivileges(result *Result, grants []string, lackPriv map[mysql.Privil if len(lackPriv[priv]) == 0 { delete(lackPriv, priv) } +======= + delete(dbPrivs.tables, tableName) +>>>>>>> a2d2a5213c (checker(dm): support wildcard in privilege checking (#7739)) } continue } @@ -260,9 +289,12 @@ func verifyPrivileges(result *Result, grants []string, lackPriv map[mysql.Privil if _, ok := lackPriv[privElem.Priv][dbName]; !ok { continue } +<<<<<<< HEAD if _, ok := lackPriv[privElem.Priv][dbName][tableName]; !ok { continue } +======= +>>>>>>> a2d2a5213c (checker(dm): support wildcard in privilege checking (#7739)) // dumpling could report error if an allow-list table is lack of privilege. // we only check that SELECT is granted on all columns, otherwise we can't SHOW CREATE TABLE if privElem.Priv == mysql.SelectPriv && len(privElem.Cols) != 0 { diff --git a/dm/pkg/checker/privilege_test.go b/dm/pkg/checker/privilege_test.go index 901b920c23a..a90c23f3cd0 100644 --- a/dm/pkg/checker/privilege_test.go +++ b/dm/pkg/checker/privilege_test.go @@ -283,9 +283,150 @@ func (t *testCheckSuite) TestVerifyReplicationPrivileges(c *tc.C) { }, } +<<<<<<< HEAD replicationPrivileges := map[mysql.PrivilegeType]struct{}{ mysql.ReplicationClientPriv: {}, mysql.ReplicationSlavePriv: {}, +======= + for _, cs := range cases { + result := &Result{ + State: StateFailure, + } + replRequiredPrivs := map[mysql.PrivilegeType]priv{ + mysql.ReplicationSlavePriv: {needGlobal: true}, + mysql.ReplicationClientPriv: {needGlobal: true}, + } + err := verifyPrivilegesWithResult(result, cs.grants, replRequiredPrivs) + if cs.replicationState == StateSuccess { + require.Nil(t, err, "grants: %v", cs.grants) + } else { + require.NotNil(t, err, "grants: %v", cs.grants) + require.Equal(t, cs.errStr, err.ShortErr, "grants: %v", cs.grants) + } + } +} + +func TestVerifyPrivilegesWildcard(t *testing.T) { + cases := []struct { + grants []string + checkTables []filter.Table + replicationState State + errStr string + }{ + { + grants: []string{ + "GRANT SELECT ON `demo\\_foobar`.* TO `dmuser`@`%`", + }, + checkTables: []filter.Table{ + {Schema: "demo_foobar", Name: "t1"}, + }, + replicationState: StateSuccess, + }, + { + grants: []string{ + "GRANT SELECT ON `demo\\_foobar`.* TO `dmuser`@`%`", + }, + checkTables: []filter.Table{ + {Schema: "demo2foobar", Name: "t1"}, + }, + replicationState: StateFailure, + errStr: "lack of Select privilege: {`demo2foobar`.`t1`}; ", + }, + { + grants: []string{ + "GRANT SELECT ON `demo_`.* TO `dmuser`@`%`", + }, + checkTables: []filter.Table{ + {Schema: "demo1", Name: "t1"}, + {Schema: "demo2", Name: "t1"}, + }, + replicationState: StateSuccess, + }, + { + grants: []string{ + "GRANT SELECT ON `demo%`.* TO `dmuser`@`%`", + }, + checkTables: []filter.Table{ + {Schema: "demo_some", Name: "t1"}, + {Schema: "block_db", Name: "t1"}, + }, + replicationState: StateFailure, + errStr: "lack of Select privilege: {`block_db`.`t1`}; ", + }, + { + grants: []string{ + "GRANT SELECT ON `demo_db`.`t1` TO `dmuser`@`%`", + }, + checkTables: []filter.Table{ + {Schema: "demo_db", Name: "t1"}, + {Schema: "demo2db", Name: "t1"}, + }, + replicationState: StateFailure, + errStr: "lack of Select privilege: {`demo2db`.`t1`}; ", + }, + } + + for i, cs := range cases { + t.Logf("case %d", i) + result := &Result{ + State: StateFailure, + } + requiredPrivs := map[mysql.PrivilegeType]priv{ + mysql.SelectPriv: { + dbs: genTableLevelPrivs(cs.checkTables), + }, + } + err := verifyPrivilegesWithResult(result, cs.grants, requiredPrivs) + if cs.replicationState == StateSuccess { + require.Nil(t, err, "grants: %v", cs.grants) + } else { + require.NotNil(t, err, "grants: %v", cs.grants) + require.Equal(t, cs.errStr, err.ShortErr, "grants: %v", cs.grants) + } + } +} + +func TestVerifyTargetPrivilege(t *testing.T) { + cases := []struct { + grants []string + checkState State + errStr string + }{ + { + grants: nil, // non grants + checkState: StateWarning, + errStr: "there is no such grant defined for current user on host '%'", + }, + { + grants: []string{"invalid SQL statement"}, + checkState: StateWarning, + errStr: "line 1 column 7 near \"invalid SQL statement\" ", + }, + { + grants: []string{"CREATE DATABASE db1"}, // non GRANT statement + checkState: StateWarning, + errStr: "CREATE DATABASE db1 is not grant statement", + }, + { + grants: []string{ + "GRANT ALL PRIVILEGES ON *.* TO 'user'@'%'", + }, + checkState: StateSuccess, + }, + { + grants: []string{ + "GRANT SELECT, CREATE, INSERT, UPDATE, DELETE, ALTER, DROP ON *.* TO 'root'@'%'", + }, + checkState: StateSuccess, + }, + { + grants: []string{ + "GRANT SELECT, INSERT, DELETE, ALTER, DROP ON *.* TO 'root'@'%'", + }, + checkState: StateWarning, + errStr: "lack of Create global (*.*) privilege; lack of Update global (*.*) privilege; ", + }, +>>>>>>> a2d2a5213c (checker(dm): support wildcard in privilege checking (#7739)) } for _, cs := range cases { result := &Result{ From 59f54a0b5145aad49ee7a58481de520d0104a256 Mon Sep 17 00:00:00 2001 From: lance6716 Date: Wed, 18 Jan 2023 15:30:44 +0800 Subject: [PATCH 2/3] fix git conflict Signed-off-by: lance6716 --- dm/pkg/checker/privilege.go | 47 +++------------- dm/pkg/checker/privilege_test.go | 96 ++++++-------------------------- 2 files changed, 26 insertions(+), 117 deletions(-) diff --git a/dm/pkg/checker/privilege.go b/dm/pkg/checker/privilege.go index 19e1e50ebf3..592b58d7a9c 100644 --- a/dm/pkg/checker/privilege.go +++ b/dm/pkg/checker/privilege.go @@ -25,15 +25,9 @@ import ( _ "github.com/pingcap/tidb/types/parser_driver" // for parser driver "github.com/pingcap/tidb/util/dbutil" "github.com/pingcap/tidb/util/filter" -<<<<<<< HEAD -======= "github.com/pingcap/tidb/util/stringutil" "github.com/pingcap/tiflow/dm/pkg/log" - "github.com/pingcap/tiflow/pkg/container/sortmap" ->>>>>>> a2d2a5213c (checker(dm): support wildcard in privilege checking (#7739)) "go.uber.org/zap" - - "github.com/pingcap/tiflow/dm/pkg/log" ) // some privileges are only effective on global level. in other words, GRANT ALL ON test.* is not enough for them @@ -209,48 +203,32 @@ func verifyPrivileges(result *Result, grants []string, lackPriv map[mysql.Privil if priv == mysql.GrantPriv { continue } -<<<<<<< HEAD - if _, ok := lackPriv[priv][dbName]; !ok { - continue + for dbName := range lackPriv[priv] { + if stringutil.DoMatch(dbName, dbPatChar, dbPatType) { + delete(lackPriv[priv], dbName) + } } - delete(lackPriv[priv], dbName) if len(lackPriv[priv]) == 0 { delete(lackPriv, priv) } -======= - for dbName := range privs.dbs { - if stringutil.DoMatch(dbName, dbPatChar, dbPatType) { - delete(privs.dbs, dbName) - } - } ->>>>>>> a2d2a5213c (checker(dm): support wildcard in privilege checking (#7739)) } continue } if _, ok := lackPriv[privElem.Priv]; !ok { continue } -<<<<<<< HEAD - if _, ok := lackPriv[privElem.Priv][dbName]; !ok { - continue - } -======= ->>>>>>> a2d2a5213c (checker(dm): support wildcard in privilege checking (#7739)) // dumpling could report error if an allow-list table is lack of privilege. // we only check that SELECT is granted on all columns, otherwise we can't SHOW CREATE TABLE if privElem.Priv == mysql.SelectPriv && len(privElem.Cols) != 0 { continue } -<<<<<<< HEAD - delete(lackPriv[privElem.Priv], dbName) - if len(lackPriv[privElem.Priv]) == 0 { - delete(lackPriv, privElem.Priv) -======= - for dbName := range privs.dbs { + for dbName := range lackPriv[privElem.Priv] { if stringutil.DoMatch(dbName, dbPatChar, dbPatType) { - delete(privs.dbs, dbName) + delete(lackPriv[privElem.Priv], dbName) } ->>>>>>> a2d2a5213c (checker(dm): support wildcard in privilege checking (#7739)) + } + if len(lackPriv[privElem.Priv]) == 0 { + delete(lackPriv, privElem.Priv) } } case ast.GrantLevelTable: @@ -266,7 +244,6 @@ func verifyPrivileges(result *Result, grants []string, lackPriv map[mysql.Privil if _, ok := lackPriv[priv][dbName]; !ok { continue } -<<<<<<< HEAD if _, ok := lackPriv[priv][dbName][tableName]; !ok { continue } @@ -277,9 +254,6 @@ func verifyPrivileges(result *Result, grants []string, lackPriv map[mysql.Privil if len(lackPriv[priv]) == 0 { delete(lackPriv, priv) } -======= - delete(dbPrivs.tables, tableName) ->>>>>>> a2d2a5213c (checker(dm): support wildcard in privilege checking (#7739)) } continue } @@ -289,12 +263,9 @@ func verifyPrivileges(result *Result, grants []string, lackPriv map[mysql.Privil if _, ok := lackPriv[privElem.Priv][dbName]; !ok { continue } -<<<<<<< HEAD if _, ok := lackPriv[privElem.Priv][dbName][tableName]; !ok { continue } -======= ->>>>>>> a2d2a5213c (checker(dm): support wildcard in privilege checking (#7739)) // dumpling could report error if an allow-list table is lack of privilege. // we only check that SELECT is granted on all columns, otherwise we can't SHOW CREATE TABLE if privElem.Priv == mysql.SelectPriv && len(privElem.Cols) != 0 { diff --git a/dm/pkg/checker/privilege_test.go b/dm/pkg/checker/privilege_test.go index a90c23f3cd0..3960345a62f 100644 --- a/dm/pkg/checker/privilege_test.go +++ b/dm/pkg/checker/privilege_test.go @@ -19,6 +19,7 @@ import ( tc "github.com/pingcap/check" "github.com/pingcap/tidb/parser/mysql" "github.com/pingcap/tidb/util/filter" + "github.com/stretchr/testify/require" ) func TestClient(t *testing.T) { @@ -283,25 +284,19 @@ func (t *testCheckSuite) TestVerifyReplicationPrivileges(c *tc.C) { }, } -<<<<<<< HEAD replicationPrivileges := map[mysql.PrivilegeType]struct{}{ mysql.ReplicationClientPriv: {}, mysql.ReplicationSlavePriv: {}, -======= + } for _, cs := range cases { result := &Result{ State: StateFailure, } - replRequiredPrivs := map[mysql.PrivilegeType]priv{ - mysql.ReplicationSlavePriv: {needGlobal: true}, - mysql.ReplicationClientPriv: {needGlobal: true}, - } - err := verifyPrivilegesWithResult(result, cs.grants, replRequiredPrivs) - if cs.replicationState == StateSuccess { - require.Nil(t, err, "grants: %v", cs.grants) - } else { - require.NotNil(t, err, "grants: %v", cs.grants) - require.Equal(t, cs.errStr, err.ShortErr, "grants: %v", cs.grants) + replicationLackGrants := genReplicPriv(replicationPrivileges) + err := verifyPrivileges(result, cs.grants, replicationLackGrants) + c.Assert(err == nil, tc.Equals, cs.replicationState == StateSuccess) + if err != nil && len(cs.errMatch) != 0 { + c.Assert(err.ShortErr, tc.Matches, cs.errMatch) } } } @@ -309,7 +304,7 @@ func (t *testCheckSuite) TestVerifyReplicationPrivileges(c *tc.C) { func TestVerifyPrivilegesWildcard(t *testing.T) { cases := []struct { grants []string - checkTables []filter.Table + checkTables []*filter.Table replicationState State errStr string }{ @@ -317,7 +312,7 @@ func TestVerifyPrivilegesWildcard(t *testing.T) { grants: []string{ "GRANT SELECT ON `demo\\_foobar`.* TO `dmuser`@`%`", }, - checkTables: []filter.Table{ + checkTables: []*filter.Table{ {Schema: "demo_foobar", Name: "t1"}, }, replicationState: StateSuccess, @@ -326,7 +321,7 @@ func TestVerifyPrivilegesWildcard(t *testing.T) { grants: []string{ "GRANT SELECT ON `demo\\_foobar`.* TO `dmuser`@`%`", }, - checkTables: []filter.Table{ + checkTables: []*filter.Table{ {Schema: "demo2foobar", Name: "t1"}, }, replicationState: StateFailure, @@ -336,7 +331,7 @@ func TestVerifyPrivilegesWildcard(t *testing.T) { grants: []string{ "GRANT SELECT ON `demo_`.* TO `dmuser`@`%`", }, - checkTables: []filter.Table{ + checkTables: []*filter.Table{ {Schema: "demo1", Name: "t1"}, {Schema: "demo2", Name: "t1"}, }, @@ -346,7 +341,7 @@ func TestVerifyPrivilegesWildcard(t *testing.T) { grants: []string{ "GRANT SELECT ON `demo%`.* TO `dmuser`@`%`", }, - checkTables: []filter.Table{ + checkTables: []*filter.Table{ {Schema: "demo_some", Name: "t1"}, {Schema: "block_db", Name: "t1"}, }, @@ -357,7 +352,7 @@ func TestVerifyPrivilegesWildcard(t *testing.T) { grants: []string{ "GRANT SELECT ON `demo_db`.`t1` TO `dmuser`@`%`", }, - checkTables: []filter.Table{ + checkTables: []*filter.Table{ {Schema: "demo_db", Name: "t1"}, {Schema: "demo2db", Name: "t1"}, }, @@ -371,12 +366,10 @@ func TestVerifyPrivilegesWildcard(t *testing.T) { result := &Result{ State: StateFailure, } - requiredPrivs := map[mysql.PrivilegeType]priv{ - mysql.SelectPriv: { - dbs: genTableLevelPrivs(cs.checkTables), - }, - } - err := verifyPrivilegesWithResult(result, cs.grants, requiredPrivs) + requiredPrivs := genExpectPriv(map[mysql.PrivilegeType]struct{}{ + mysql.SelectPriv: {}, + }, cs.checkTables) + err := verifyPrivileges(result, cs.grants, requiredPrivs) if cs.replicationState == StateSuccess { require.Nil(t, err, "grants: %v", cs.grants) } else { @@ -385,58 +378,3 @@ func TestVerifyPrivilegesWildcard(t *testing.T) { } } } - -func TestVerifyTargetPrivilege(t *testing.T) { - cases := []struct { - grants []string - checkState State - errStr string - }{ - { - grants: nil, // non grants - checkState: StateWarning, - errStr: "there is no such grant defined for current user on host '%'", - }, - { - grants: []string{"invalid SQL statement"}, - checkState: StateWarning, - errStr: "line 1 column 7 near \"invalid SQL statement\" ", - }, - { - grants: []string{"CREATE DATABASE db1"}, // non GRANT statement - checkState: StateWarning, - errStr: "CREATE DATABASE db1 is not grant statement", - }, - { - grants: []string{ - "GRANT ALL PRIVILEGES ON *.* TO 'user'@'%'", - }, - checkState: StateSuccess, - }, - { - grants: []string{ - "GRANT SELECT, CREATE, INSERT, UPDATE, DELETE, ALTER, DROP ON *.* TO 'root'@'%'", - }, - checkState: StateSuccess, - }, - { - grants: []string{ - "GRANT SELECT, INSERT, DELETE, ALTER, DROP ON *.* TO 'root'@'%'", - }, - checkState: StateWarning, - errStr: "lack of Create global (*.*) privilege; lack of Update global (*.*) privilege; ", - }, ->>>>>>> a2d2a5213c (checker(dm): support wildcard in privilege checking (#7739)) - } - for _, cs := range cases { - result := &Result{ - State: StateFailure, - } - replicationLackGrants := genReplicPriv(replicationPrivileges) - err := verifyPrivileges(result, cs.grants, replicationLackGrants) - c.Assert(err == nil, tc.Equals, cs.replicationState == StateSuccess) - if err != nil && len(cs.errMatch) != 0 { - c.Assert(err.ShortErr, tc.Matches, cs.errMatch) - } - } -} From a4832782418458c28a2dcd850d55edd8f9212248 Mon Sep 17 00:00:00 2001 From: lance6716 Date: Wed, 18 Jan 2023 16:36:56 +0800 Subject: [PATCH 3/3] fix test Signed-off-by: lance6716 --- dm/pkg/checker/privilege.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/dm/pkg/checker/privilege.go b/dm/pkg/checker/privilege.go index 592b58d7a9c..b04f6b765d2 100644 --- a/dm/pkg/checker/privilege.go +++ b/dm/pkg/checker/privilege.go @@ -203,12 +203,16 @@ func verifyPrivileges(result *Result, grants []string, lackPriv map[mysql.Privil if priv == mysql.GrantPriv { continue } + // in this old release branch, we don't have a flag that mark a privilege is global level. + // we use `deleted` to avoid delete a global level privilege when processing AllPriv. + deleted := false for dbName := range lackPriv[priv] { if stringutil.DoMatch(dbName, dbPatChar, dbPatType) { delete(lackPriv[priv], dbName) } + deleted = true } - if len(lackPriv[priv]) == 0 { + if deleted && len(lackPriv[priv]) == 0 { delete(lackPriv, priv) } }