From 8bdcd762c719008ae242b0d0429cbd94fc425b4c Mon Sep 17 00:00:00 2001 From: unconsolable Date: Mon, 1 Nov 2021 20:56:10 +0800 Subject: [PATCH 1/5] executor: fix error msg of granting non-table level privilege to a non-existent table Signed-off-by: unconsolable --- executor/grant.go | 18 ++++++++++-------- executor/grant_test.go | 14 ++++++++++++++ privilege/privileges/privileges_test.go | 1 + 3 files changed, 25 insertions(+), 8 deletions(-) diff --git a/executor/grant.go b/executor/grant.go index 7e0c1703d444c..edae072eb31d5 100644 --- a/executor/grant.go +++ b/executor/grant.go @@ -72,8 +72,17 @@ func (e *GrantExec) Next(ctx context.Context, req *chunk.Chunk) error { dbName = e.ctx.GetSessionVars().CurrentDB } - // Make sure the table exist. + // For table , check whether table exists and privilege is valid if e.Level.Level == ast.GrantLevelTable { + // Return if privilege is invalid, to fail before not existing table, see issue #29302 + for _, p := range e.Privs { + // TODO: https://github.com/pingcap/parser/pull/581 removed privs from all priv lists + // it is to avoid add GRANT in GRANT ALL SQLs + // WithGRANT seems broken, fix it later + if p.Priv != mysql.GrantPriv && !mysql.AllTablePrivs.Has(p.Priv) { + return ErrIllegalGrantForTable + } + } dbNameStr := model.NewCIStr(dbName) schema := e.ctx.GetInfoSchema().(infoschema.InfoSchema) tbl, err := schema.TableByName(dbNameStr, model.NewCIStr(e.Level.TableName)) @@ -633,13 +642,6 @@ func composeDBPrivUpdate(sql *strings.Builder, priv mysql.PrivilegeType, value s func composeTablePrivUpdateForGrant(ctx sessionctx.Context, sql *strings.Builder, priv mysql.PrivilegeType, name string, host string, db string, tbl string) error { var newTablePriv, newColumnPriv []string if priv != mysql.AllPriv { - // TODO: https://github.com/pingcap/parser/pull/581 removed privs from all priv lists - // it is to avoid add GRANT in GRANT ALL SQLs - // WithGRANT seems broken, fix it later - if priv != mysql.GrantPriv && !mysql.AllTablePrivs.Has(priv) { - return ErrIllegalGrantForTable - } - currTablePriv, currColumnPriv, err := getTablePriv(ctx, name, host, db, tbl) if err != nil { return err diff --git a/executor/grant_test.go b/executor/grant_test.go index 906d87e0b5cd2..80164bdfd219d 100644 --- a/executor/grant_test.go +++ b/executor/grant_test.go @@ -615,3 +615,17 @@ func TestGrantDynamicPrivs(t *testing.T) { tk.MustQuery("SELECT Grant_Priv FROM mysql.user WHERE `Host` = '%' AND `User` = 'dyn'").Check(testkit.Rows("Y")) tk.MustQuery("SELECT WITH_GRANT_OPTION FROM mysql.global_grants WHERE `Host` = '%' AND `User` = 'dyn' AND Priv='CONNECTION_ADMIN'").Check(testkit.Rows("Y")) } + +func TestNonExistTableIllegalGrant(t *testing.T) { + t.Parallel() + + store, clean := testkit.CreateMockStore(t) + defer clean() + + tk := testkit.NewTestKit(t, store) + tk.MustExec("create user i29302") + defer tk.MustExec("drop user i29302") + + tk.MustGetErrCode("grant create temporary tables on tmpdb.tmp29302 to test_user", mysql.ErrIllegalGrantForTable) + tk.MustGetErrCode("grant lock tables on test.tmp29302 to test_user", mysql.ErrIllegalGrantForTable) +} diff --git a/privilege/privileges/privileges_test.go b/privilege/privileges/privileges_test.go index a85a33e41eda9..11cd124846fb9 100644 --- a/privilege/privileges/privileges_test.go +++ b/privilege/privileges/privileges_test.go @@ -2667,6 +2667,7 @@ func TestGrantCreateTmpTables(t *testing.T) { tk.MustExec("CREATE TABLE create_tmp_table_table (a int)") tk.MustExec("GRANT CREATE TEMPORARY TABLES on create_tmp_table_db.* to u1") tk.MustExec("GRANT CREATE TEMPORARY TABLES on *.* to u1") + tk.MustGetErrCode("GRANT CREATE TEMPORARY TABLES on create_tmp_table_db.tmp to u1", mysql.ErrIllegalGrantForTable) // Must set a session user to avoid null pointer dereference tk.Session().Auth(&auth.UserIdentity{ Username: "root", From 275d4b8a69a777b6016f756c5129ed4d1631e317 Mon Sep 17 00:00:00 2001 From: unconsolable Date: Mon, 1 Nov 2021 21:00:52 +0800 Subject: [PATCH 2/5] adapt reviewer's comment, fix test --- executor/grant.go | 6 ++---- executor/grant_test.go | 8 ++++---- parser/mysql/privs.go | 3 +++ 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/executor/grant.go b/executor/grant.go index edae072eb31d5..d39d1495d91b3 100644 --- a/executor/grant.go +++ b/executor/grant.go @@ -76,10 +76,8 @@ func (e *GrantExec) Next(ctx context.Context, req *chunk.Chunk) error { if e.Level.Level == ast.GrantLevelTable { // Return if privilege is invalid, to fail before not existing table, see issue #29302 for _, p := range e.Privs { - // TODO: https://github.com/pingcap/parser/pull/581 removed privs from all priv lists - // it is to avoid add GRANT in GRANT ALL SQLs - // WithGRANT seems broken, fix it later - if p.Priv != mysql.GrantPriv && !mysql.AllTablePrivs.Has(p.Priv) { + // Exclude column level + if len(p.Cols) == 0 && !mysql.AllowedTablePrivs.Has(p.Priv) { return ErrIllegalGrantForTable } } diff --git a/executor/grant_test.go b/executor/grant_test.go index 80164bdfd219d..7e9c4120fdacb 100644 --- a/executor/grant_test.go +++ b/executor/grant_test.go @@ -623,9 +623,9 @@ func TestNonExistTableIllegalGrant(t *testing.T) { defer clean() tk := testkit.NewTestKit(t, store) - tk.MustExec("create user i29302") - defer tk.MustExec("drop user i29302") + tk.MustExec("create user u29302") + defer tk.MustExec("drop user u29302") - tk.MustGetErrCode("grant create temporary tables on tmpdb.tmp29302 to test_user", mysql.ErrIllegalGrantForTable) - tk.MustGetErrCode("grant lock tables on test.tmp29302 to test_user", mysql.ErrIllegalGrantForTable) + tk.MustGetErrCode("grant create temporary tables on NotExistsD29302.NotExistsT29302 to u29302", mysql.ErrIllegalGrantForTable) + tk.MustGetErrCode("grant lock tables on test.NotExistsT29302 to u29302", mysql.ErrIllegalGrantForTable) } diff --git a/parser/mysql/privs.go b/parser/mysql/privs.go index 93e5db579ad1e..8662557b5bd88 100644 --- a/parser/mysql/privs.go +++ b/parser/mysql/privs.go @@ -314,6 +314,9 @@ var AllDBPrivs = Privileges{SelectPriv, InsertPriv, UpdatePriv, DeletePriv, Crea // AllTablePrivs is all the privileges in table scope. var AllTablePrivs = Privileges{SelectPriv, InsertPriv, UpdatePriv, DeletePriv, CreatePriv, DropPriv, IndexPriv, ReferencesPriv, AlterPriv, CreateViewPriv, ShowViewPriv} +// AllowedTablePrivs is all the privileges allowed in table scope. +var AllowedTablePrivs = Privileges{SelectPriv, InsertPriv, UpdatePriv, DeletePriv, CreatePriv, DropPriv, IndexPriv, ReferencesPriv, AlterPriv, CreateViewPriv, ShowViewPriv, GrantPriv, UsagePriv, AllPriv} + // AllColumnPrivs is all the privileges in column scope. var AllColumnPrivs = Privileges{SelectPriv, InsertPriv, UpdatePriv, ReferencesPriv} From 4bb9a0c2d7dc2358d3e7f82ea33302b5b86becfa Mon Sep 17 00:00:00 2001 From: unconsolable Date: Thu, 4 Nov 2021 19:07:42 +0800 Subject: [PATCH 3/5] refactor column privilege check Signed-off-by: unconsolable --- executor/grant.go | 17 +++++++++-------- parser/mysql/privs.go | 3 --- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/executor/grant.go b/executor/grant.go index d39d1495d91b3..9f0d294fc1f49 100644 --- a/executor/grant.go +++ b/executor/grant.go @@ -72,13 +72,18 @@ func (e *GrantExec) Next(ctx context.Context, req *chunk.Chunk) error { dbName = e.ctx.GetSessionVars().CurrentDB } - // For table , check whether table exists and privilege is valid + // For table & column level, check whether table exists and privilege is valid if e.Level.Level == ast.GrantLevelTable { // Return if privilege is invalid, to fail before not existing table, see issue #29302 for _, p := range e.Privs { - // Exclude column level - if len(p.Cols) == 0 && !mysql.AllowedTablePrivs.Has(p.Priv) { - return ErrIllegalGrantForTable + if len(p.Cols) == 0 { + if !mysql.AllTablePrivs.Has(p.Priv) && p.Priv != mysql.AllPriv && p.Priv != mysql.UsagePriv && p.Priv != mysql.GrantPriv { + return ErrIllegalGrantForTable + } + } else { + if !mysql.AllColumnPrivs.Has(p.Priv) && p.Priv != mysql.AllPriv && p.Priv != mysql.UsagePriv { + return ErrWrongUsage.GenWithStackByArgs("COLUMN GRANT", "NON-COLUMN PRIVILEGES") + } } } dbNameStr := model.NewCIStr(dbName) @@ -669,10 +674,6 @@ func composeTablePrivUpdateForGrant(ctx sessionctx.Context, sql *strings.Builder func composeColumnPrivUpdateForGrant(ctx sessionctx.Context, sql *strings.Builder, priv mysql.PrivilegeType, name string, host string, db string, tbl string, col string) error { var newColumnPriv []string if priv != mysql.AllPriv { - if !mysql.AllColumnPrivs.Has(priv) { - return ErrWrongUsage.GenWithStackByArgs("COLUMN GRANT", "NON-COLUMN PRIVILEGES") - } - currColumnPriv, err := getColumnPriv(ctx, name, host, db, tbl, col) if err != nil { return err diff --git a/parser/mysql/privs.go b/parser/mysql/privs.go index 8662557b5bd88..93e5db579ad1e 100644 --- a/parser/mysql/privs.go +++ b/parser/mysql/privs.go @@ -314,9 +314,6 @@ var AllDBPrivs = Privileges{SelectPriv, InsertPriv, UpdatePriv, DeletePriv, Crea // AllTablePrivs is all the privileges in table scope. var AllTablePrivs = Privileges{SelectPriv, InsertPriv, UpdatePriv, DeletePriv, CreatePriv, DropPriv, IndexPriv, ReferencesPriv, AlterPriv, CreateViewPriv, ShowViewPriv} -// AllowedTablePrivs is all the privileges allowed in table scope. -var AllowedTablePrivs = Privileges{SelectPriv, InsertPriv, UpdatePriv, DeletePriv, CreatePriv, DropPriv, IndexPriv, ReferencesPriv, AlterPriv, CreateViewPriv, ShowViewPriv, GrantPriv, UsagePriv, AllPriv} - // AllColumnPrivs is all the privileges in column scope. var AllColumnPrivs = Privileges{SelectPriv, InsertPriv, UpdatePriv, ReferencesPriv} From 02b70376d01444ed592efe458d6c23a293ddd685 Mon Sep 17 00:00:00 2001 From: unconsolable Date: Thu, 4 Nov 2021 19:54:45 +0800 Subject: [PATCH 4/5] refine test. Signed-off-by: unconsolable --- executor/grant_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/executor/grant_test.go b/executor/grant_test.go index 7e9c4120fdacb..a42b546e42b05 100644 --- a/executor/grant_test.go +++ b/executor/grant_test.go @@ -625,7 +625,9 @@ func TestNonExistTableIllegalGrant(t *testing.T) { tk := testkit.NewTestKit(t, store) tk.MustExec("create user u29302") defer tk.MustExec("drop user u29302") - + // Table level, not existing table, illegal privilege tk.MustGetErrCode("grant create temporary tables on NotExistsD29302.NotExistsT29302 to u29302", mysql.ErrIllegalGrantForTable) tk.MustGetErrCode("grant lock tables on test.NotExistsT29302 to u29302", mysql.ErrIllegalGrantForTable) + // Column level, not existing table, illegal privilege + tk.MustGetErrCode("grant create temporary tables (NotExistsCol) on NotExistsD29302.NotExistsT29302 to u29302;", mysql.ErrWrongUsage) } From ed6d3720586524048edc8dc4a76fad7db47e54ea Mon Sep 17 00:00:00 2001 From: unconsolable Date: Sat, 13 Nov 2021 10:20:23 +0800 Subject: [PATCH 5/5] fix dynamic privilege fail Signed-off-by: unconsolable --- executor/grant.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/executor/grant.go b/executor/grant.go index c991c453bc9a5..e383f76d76bf3 100644 --- a/executor/grant.go +++ b/executor/grant.go @@ -77,7 +77,7 @@ func (e *GrantExec) Next(ctx context.Context, req *chunk.Chunk) error { // Return if privilege is invalid, to fail before not existing table, see issue #29302 for _, p := range e.Privs { if len(p.Cols) == 0 { - if !mysql.AllTablePrivs.Has(p.Priv) && p.Priv != mysql.AllPriv && p.Priv != mysql.UsagePriv && p.Priv != mysql.GrantPriv { + if !mysql.AllTablePrivs.Has(p.Priv) && p.Priv != mysql.AllPriv && p.Priv != mysql.UsagePriv && p.Priv != mysql.GrantPriv && p.Priv != mysql.ExtendedPriv { return ErrIllegalGrantForTable } } else {