From 688bfaf7aeece5182520142618350ef14087d30d Mon Sep 17 00:00:00 2001 From: imtbkcat Date: Wed, 4 Dec 2019 14:21:26 +0800 Subject: [PATCH 1/5] fix grant role privilege --- planner/core/planbuilder.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/planner/core/planbuilder.go b/planner/core/planbuilder.go index 650361cd67ffe..d6126d4ed3c88 100644 --- a/planner/core/planbuilder.go +++ b/planner/core/planbuilder.go @@ -1621,7 +1621,7 @@ func (b *PlanBuilder) buildSimple(node ast.StmtNode) (Plan, error) { b.visitInfo = collectVisitInfoFromGrantStmt(b.ctx, b.visitInfo, raw) case *ast.GrantRoleStmt: err := ErrSpecificAccessDenied.GenWithStackByArgs("GRANT ROLE") - b.visitInfo = appendVisitInfo(b.visitInfo, mysql.GrantPriv, "", "", "", err) + b.visitInfo = appendVisitInfo(b.visitInfo, mysql.SuperPriv, "", "", "", err) case *ast.RevokeStmt: b.visitInfo = collectVisitInfoFromRevokeStmt(b.ctx, b.visitInfo, raw) case *ast.RevokeRoleStmt: From 122cf34b6f950ce975c44981799b3a2febf807a3 Mon Sep 17 00:00:00 2001 From: imtbkcat Date: Wed, 4 Dec 2019 14:22:41 +0800 Subject: [PATCH 2/5] fix error message --- planner/core/planbuilder.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/planner/core/planbuilder.go b/planner/core/planbuilder.go index d6126d4ed3c88..5b1a4a47df589 100644 --- a/planner/core/planbuilder.go +++ b/planner/core/planbuilder.go @@ -1620,11 +1620,12 @@ func (b *PlanBuilder) buildSimple(node ast.StmtNode) (Plan, error) { } b.visitInfo = collectVisitInfoFromGrantStmt(b.ctx, b.visitInfo, raw) case *ast.GrantRoleStmt: - err := ErrSpecificAccessDenied.GenWithStackByArgs("GRANT ROLE") + err := ErrSpecificAccessDenied.GenWithStackByArgs("SUPER") b.visitInfo = appendVisitInfo(b.visitInfo, mysql.SuperPriv, "", "", "", err) case *ast.RevokeStmt: b.visitInfo = collectVisitInfoFromRevokeStmt(b.ctx, b.visitInfo, raw) case *ast.RevokeRoleStmt: + err := ErrSpecificAccessDenied.GenWithStackByArgs("SUPER") b.visitInfo = appendVisitInfo(b.visitInfo, mysql.SuperPriv, "", "", "", nil) case *ast.KillStmt: // If you have the SUPER privilege, you can kill all threads and statements. From 84d6033163c643d3e6351c932937fa2ebd5bee64 Mon Sep 17 00:00:00 2001 From: imtbkcat Date: Wed, 4 Dec 2019 14:23:05 +0800 Subject: [PATCH 3/5] fix error --- planner/core/planbuilder.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/planner/core/planbuilder.go b/planner/core/planbuilder.go index 5b1a4a47df589..3188720f40c75 100644 --- a/planner/core/planbuilder.go +++ b/planner/core/planbuilder.go @@ -1626,7 +1626,7 @@ func (b *PlanBuilder) buildSimple(node ast.StmtNode) (Plan, error) { b.visitInfo = collectVisitInfoFromRevokeStmt(b.ctx, b.visitInfo, raw) case *ast.RevokeRoleStmt: err := ErrSpecificAccessDenied.GenWithStackByArgs("SUPER") - b.visitInfo = appendVisitInfo(b.visitInfo, mysql.SuperPriv, "", "", "", nil) + b.visitInfo = appendVisitInfo(b.visitInfo, mysql.SuperPriv, "", "", "", err) case *ast.KillStmt: // If you have the SUPER privilege, you can kill all threads and statements. // Otherwise, you can kill only your own threads and statements. From c8c448e181fff3dbd1533d7e479b4835bc06b646 Mon Sep 17 00:00:00 2001 From: imtbkcat Date: Wed, 4 Dec 2019 15:40:39 +0800 Subject: [PATCH 4/5] add test --- executor/simple_test.go | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/executor/simple_test.go b/executor/simple_test.go index f237c494b6284..cfa068eb27390 100644 --- a/executor/simple_test.go +++ b/executor/simple_test.go @@ -188,6 +188,28 @@ func (s *testSuite3) TestRole(c *C) { tk.MustExec("SET ROLE NONE") } +func (s *testSuite3) TestRoleAdmin(c *C) { + tk := testkit.NewTestKit(c, s.store) + tk.MustExec("CREATE USER 'testRoleAdmin';") + tk.MustExec("CREATE ROLE 'targetRole';") + tk.MustExec("GRANT SUPER ON *.* TO `testRoleAdmin`;") + + // Create a new session. + se, err := session.CreateSession4Test(s.store) + c.Check(err, IsNil) + defer se.Close() + c.Assert(se.Auth(&auth.UserIdentity{Username: "testRoleAdmin", Hostname: "localhost"}, nil, nil), IsTrue) + + ctx := context.Background() + _, err = se.Execute(ctx, "GRANT `targetRole` TO `testRoleAdmin`;") + c.Assert(err, IsNil) + _, err = se.Execute(ctx, "REVOKE `targetRole` FROM `testRoleAdmin`;") + c.Assert(err, IsNil) + + tk.MustExec("DROP USER 'testRoleAdmin';") + tk.MustExec("DROP ROLE 'targetRole';") +} + func (s *testSuite3) TestDefaultRole(c *C) { tk := testkit.NewTestKit(c, s.store) From 5ce7af4ca5e998082f217727cb228750a235d60b Mon Sep 17 00:00:00 2001 From: imtbkcat Date: Wed, 4 Dec 2019 19:46:11 +0800 Subject: [PATCH 5/5] add failed case --- executor/simple_test.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/executor/simple_test.go b/executor/simple_test.go index cfa068eb27390..1fe1fbba14d80 100644 --- a/executor/simple_test.go +++ b/executor/simple_test.go @@ -192,7 +192,6 @@ func (s *testSuite3) TestRoleAdmin(c *C) { tk := testkit.NewTestKit(c, s.store) tk.MustExec("CREATE USER 'testRoleAdmin';") tk.MustExec("CREATE ROLE 'targetRole';") - tk.MustExec("GRANT SUPER ON *.* TO `testRoleAdmin`;") // Create a new session. se, err := session.CreateSession4Test(s.store) @@ -202,6 +201,10 @@ func (s *testSuite3) TestRoleAdmin(c *C) { ctx := context.Background() _, err = se.Execute(ctx, "GRANT `targetRole` TO `testRoleAdmin`;") + c.Assert(err, NotNil) + + tk.MustExec("GRANT SUPER ON *.* TO `testRoleAdmin`;") + _, err = se.Execute(ctx, "GRANT `targetRole` TO `testRoleAdmin`;") c.Assert(err, IsNil) _, err = se.Execute(ctx, "REVOKE `targetRole` FROM `testRoleAdmin`;") c.Assert(err, IsNil)