From 56c6d212a3175762addee19744b3e315bdbf03f7 Mon Sep 17 00:00:00 2001 From: Hangjie Mo Date: Thu, 9 Feb 2023 11:34:02 +0800 Subject: [PATCH 1/4] planner: recover table/schema only needs create and drop privilege --- executor/recover_test.go | 57 +++++++++++++++++++++++++++++++++++++ planner/core/planbuilder.go | 32 +++++++++++++++++++-- 2 files changed, 86 insertions(+), 3 deletions(-) diff --git a/executor/recover_test.go b/executor/recover_test.go index 806e251f17769..6f43fdca59d55 100644 --- a/executor/recover_test.go +++ b/executor/recover_test.go @@ -296,6 +296,43 @@ func TestRecoverTableMeetError(t *testing.T) { tk.MustContainErrMsg("select * from t_recover", "Table 'test_recover.t_recover' doesn't exist") } +func TestRecoverTablePrivilege(t *testing.T) { + store := testkit.CreateMockStore(t) + tk := testkit.NewTestKit(t, store) + + timeBeforeDrop, _, safePointSQL, resetGC := MockGC(tk) + defer resetGC() + + // Set GC safe point + tk.MustExec(fmt.Sprintf(safePointSQL, timeBeforeDrop)) + + tk.MustExec("use test") + tk.MustExec("drop table if exists t_recover") + tk.MustExec("create table t_recover (a int);") + tk.MustExec("drop table t_recover") + + // Recover without drop/create privilege. + tk.MustExec("CREATE USER 'testrecovertable'@'localhost';") + newTk := testkit.NewTestKit(t, store) + require.NoError(t, newTk.Session().Auth(&auth.UserIdentity{Username: "testrecovertable", Hostname: "localhost"}, nil, nil)) + newTk.MustGetErrCode("recover table t_recover", errno.ErrTableaccessDenied) + newTk.MustGetErrCode("flashback table t_recover", errno.ErrTableaccessDenied) + + // Got drop privilege, still failed. + tk.MustExec("grant drop on *.* to 'testrecovertable'@'localhost';") + newTk.MustGetErrCode("recover table t_recover", errno.ErrTableaccessDenied) + newTk.MustGetErrCode("flashback table t_recover", errno.ErrTableaccessDenied) + + // Got select, create and drop privilege, execute success. + tk.MustExec("grant select,create on *.* to 'testrecovertable'@'localhost';") + newTk.MustExec("use test") + newTk.MustExec("recover table t_recover") + newTk.MustExec("drop table t_recover") + newTk.MustExec("flashback table t_recover") + + tk.MustExec("drop user 'testrecovertable'@'localhost';") +} + func TestRecoverClusterMeetError(t *testing.T) { store := testkit.CreateMockStore(t) tk := testkit.NewTestKit(t, store) @@ -499,6 +536,26 @@ func TestFlashbackSchema(t *testing.T) { tk.MustExec("use test2") tk.MustQuery("select a from t order by a").Check(testkit.Rows("1", "2", "3")) tk.MustQuery("select a from t1 order by a").Check(testkit.Rows("4", "5", "6")) + + tk.MustExec("drop database if exists t_recover") + tk.MustExec("create database t_recover") + tk.MustExec("drop database t_recover") + + // Recover without drop/create privilege. + tk.MustExec("CREATE USER 'testflashbackschema'@'localhost';") + newTk := testkit.NewTestKit(t, store) + require.NoError(t, newTk.Session().Auth(&auth.UserIdentity{Username: "testflashbackschema", Hostname: "localhost"}, nil, nil)) + newTk.MustGetErrCode("flashback database t_recover", errno.ErrDBaccessDenied) + + // Got drop privilege, still failed. + tk.MustExec("grant drop on *.* to 'testflashbackschema'@'localhost';") + newTk.MustGetErrCode("flashback database t_recover", errno.ErrDBaccessDenied) + + // Got select, create and drop privilege, execute success. + tk.MustExec("grant create on *.* to 'testflashbackschema'@'localhost';") + newTk.MustExec("flashback schema t_recover") + + tk.MustExec("drop user 'testflashbackschema'@'localhost';") } // MockGC is used to make GC work in the test environment. diff --git a/planner/core/planbuilder.go b/planner/core/planbuilder.go index 72e2cc048bf55..828fe9d728ed5 100644 --- a/planner/core/planbuilder.go +++ b/planner/core/planbuilder.go @@ -4727,9 +4727,35 @@ func (b *PlanBuilder) buildDDL(ctx context.Context, node ast.DDLNode) (Plan, err } b.visitInfo = appendVisitInfo(b.visitInfo, mysql.InsertPriv, v.TableToTables[0].NewTable.Schema.L, v.TableToTables[0].NewTable.Name.L, "", authErr) - case *ast.RecoverTableStmt, *ast.FlashBackTableStmt, *ast.FlashBackDatabaseStmt: - // Recover table command can only be executed by administrator. - b.visitInfo = appendVisitInfo(b.visitInfo, mysql.SuperPriv, "", "", "", nil) + case *ast.RecoverTableStmt: + if b.ctx.GetSessionVars().User != nil { + authErr = ErrTableaccessDenied.GenWithStackByArgs("CREATE", b.ctx.GetSessionVars().User.AuthUsername, + b.ctx.GetSessionVars().User.AuthHostname, v.Table.Name.L) + } + b.visitInfo = appendVisitInfo(b.visitInfo, mysql.CreatePriv, v.Table.Schema.L, v.Table.Name.L, "", authErr) + if b.ctx.GetSessionVars().User != nil { + authErr = ErrTableaccessDenied.GenWithStackByArgs("DROP", b.ctx.GetSessionVars().User.AuthUsername, + b.ctx.GetSessionVars().User.AuthHostname, v.Table.Name.L) + } + b.visitInfo = appendVisitInfo(b.visitInfo, mysql.DropPriv, v.Table.Schema.L, v.Table.Name.L, "", authErr) + case *ast.FlashBackTableStmt: + if b.ctx.GetSessionVars().User != nil { + authErr = ErrTableaccessDenied.GenWithStackByArgs("CREATE", b.ctx.GetSessionVars().User.AuthUsername, + b.ctx.GetSessionVars().User.AuthHostname, v.Table.Name.L) + } + b.visitInfo = appendVisitInfo(b.visitInfo, mysql.CreatePriv, v.Table.Schema.L, v.Table.Name.L, "", authErr) + if b.ctx.GetSessionVars().User != nil { + authErr = ErrTableaccessDenied.GenWithStackByArgs("DROP", b.ctx.GetSessionVars().User.AuthUsername, + b.ctx.GetSessionVars().User.AuthHostname, v.Table.Name.L) + } + b.visitInfo = appendVisitInfo(b.visitInfo, mysql.DropPriv, v.Table.Schema.L, v.Table.Name.L, "", authErr) + case *ast.FlashBackDatabaseStmt: + if b.ctx.GetSessionVars().User != nil { + authErr = ErrDBaccessDenied.GenWithStackByArgs(b.ctx.GetSessionVars().User.AuthUsername, + b.ctx.GetSessionVars().User.AuthHostname, v.DBName.L) + } + b.visitInfo = appendVisitInfo(b.visitInfo, mysql.CreatePriv, v.DBName.L, "", "", authErr) + b.visitInfo = appendVisitInfo(b.visitInfo, mysql.DropPriv, v.DBName.L, "", "", authErr) case *ast.FlashBackToTimestampStmt: // Flashback cluster can only be executed by user with `super` privilege. b.visitInfo = appendVisitInfo(b.visitInfo, mysql.SuperPriv, "", "", "", nil) From bb9a426a1408d65db9ad1b030dc011721b1a1786 Mon Sep 17 00:00:00 2001 From: Hangjie Mo Date: Thu, 9 Feb 2023 15:11:55 +0800 Subject: [PATCH 2/4] Update recover_test.go --- executor/recover_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/executor/recover_test.go b/executor/recover_test.go index 6f43fdca59d55..92aa83ec99d3e 100644 --- a/executor/recover_test.go +++ b/executor/recover_test.go @@ -551,7 +551,7 @@ func TestFlashbackSchema(t *testing.T) { tk.MustExec("grant drop on *.* to 'testflashbackschema'@'localhost';") newTk.MustGetErrCode("flashback database t_recover", errno.ErrDBaccessDenied) - // Got select, create and drop privilege, execute success. + // Got create and drop privilege, execute success. tk.MustExec("grant create on *.* to 'testflashbackschema'@'localhost';") newTk.MustExec("flashback schema t_recover") From 629ec9fcf5c2ebdb1e44ef3a895ba3ba0b99a428 Mon Sep 17 00:00:00 2001 From: Jason Mo Date: Fri, 10 Feb 2023 12:16:19 +0800 Subject: [PATCH 3/4] fix test --- planner/core/planbuilder.go | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/planner/core/planbuilder.go b/planner/core/planbuilder.go index 828fe9d728ed5..fc5ee481aba76 100644 --- a/planner/core/planbuilder.go +++ b/planner/core/planbuilder.go @@ -4728,16 +4728,20 @@ func (b *PlanBuilder) buildDDL(ctx context.Context, node ast.DDLNode) (Plan, err b.visitInfo = appendVisitInfo(b.visitInfo, mysql.InsertPriv, v.TableToTables[0].NewTable.Schema.L, v.TableToTables[0].NewTable.Name.L, "", authErr) case *ast.RecoverTableStmt: - if b.ctx.GetSessionVars().User != nil { - authErr = ErrTableaccessDenied.GenWithStackByArgs("CREATE", b.ctx.GetSessionVars().User.AuthUsername, - b.ctx.GetSessionVars().User.AuthHostname, v.Table.Name.L) - } - b.visitInfo = appendVisitInfo(b.visitInfo, mysql.CreatePriv, v.Table.Schema.L, v.Table.Name.L, "", authErr) - if b.ctx.GetSessionVars().User != nil { - authErr = ErrTableaccessDenied.GenWithStackByArgs("DROP", b.ctx.GetSessionVars().User.AuthUsername, - b.ctx.GetSessionVars().User.AuthHostname, v.Table.Name.L) + if v.Table == nil { + b.visitInfo = appendVisitInfo(b.visitInfo, mysql.SuperPriv, "", "", "", nil) + } else { + if b.ctx.GetSessionVars().User != nil { + authErr = ErrTableaccessDenied.GenWithStackByArgs("CREATE", b.ctx.GetSessionVars().User.AuthUsername, + b.ctx.GetSessionVars().User.AuthHostname, v.Table.Name.L) + } + b.visitInfo = appendVisitInfo(b.visitInfo, mysql.CreatePriv, v.Table.Schema.L, v.Table.Name.L, "", authErr) + if b.ctx.GetSessionVars().User != nil { + authErr = ErrTableaccessDenied.GenWithStackByArgs("DROP", b.ctx.GetSessionVars().User.AuthUsername, + b.ctx.GetSessionVars().User.AuthHostname, v.Table.Name.L) + } + b.visitInfo = appendVisitInfo(b.visitInfo, mysql.DropPriv, v.Table.Schema.L, v.Table.Name.L, "", authErr) } - b.visitInfo = appendVisitInfo(b.visitInfo, mysql.DropPriv, v.Table.Schema.L, v.Table.Name.L, "", authErr) case *ast.FlashBackTableStmt: if b.ctx.GetSessionVars().User != nil { authErr = ErrTableaccessDenied.GenWithStackByArgs("CREATE", b.ctx.GetSessionVars().User.AuthUsername, From 4805c4500c9a475f0cdb3978baf20b059fe2885d Mon Sep 17 00:00:00 2001 From: Jason Mo Date: Fri, 10 Feb 2023 17:04:47 +0800 Subject: [PATCH 4/4] close race on sessiontxn first --- sessiontxn/BUILD.bazel | 1 - 1 file changed, 1 deletion(-) diff --git a/sessiontxn/BUILD.bazel b/sessiontxn/BUILD.bazel index 4691bfa4ff3af..b37a5e4ec04d3 100644 --- a/sessiontxn/BUILD.bazel +++ b/sessiontxn/BUILD.bazel @@ -27,7 +27,6 @@ go_test( "txn_rc_tso_optimize_test.go", ], flaky = True, - race = "on", shard_count = 50, deps = [ ":sessiontxn",