From 21e99e327d5b0cd036e9ed748f6e0320e8c6d177 Mon Sep 17 00:00:00 2001
From: Lingyu Song <songlingyu@pingcap.com>
Date: Wed, 11 Dec 2019 13:43:58 +0800
Subject: [PATCH] privilege: fix privilege check of `CREATE ROLE` and `DROP
 ROLE` (#13940)

---
 executor/simple.go      | 19 +++++++++++-----
 executor/simple_test.go | 48 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 62 insertions(+), 5 deletions(-)

diff --git a/executor/simple.go b/executor/simple.go
index a26f4510e3a00..4442ab9bbed76 100644
--- a/executor/simple.go
+++ b/executor/simple.go
@@ -644,8 +644,11 @@ func (e *SimpleExec) executeCreateUser(ctx context.Context, s *ast.CreateUserStm
 		}
 		activeRoles := e.ctx.GetSessionVars().ActiveRoles
 		if !checker.RequestVerification(activeRoles, mysql.SystemDB, mysql.UserTable, "", mysql.InsertPriv) {
-			if s.IsCreateRole && !checker.RequestVerification(activeRoles, "", "", "", mysql.CreateRolePriv) {
-				return core.ErrSpecificAccessDenied.GenWithStackByArgs("CREATE ROLE")
+			if s.IsCreateRole {
+				if !checker.RequestVerification(activeRoles, "", "", "", mysql.CreateRolePriv) &&
+					!checker.RequestVerification(activeRoles, "", "", "", mysql.CreateUserPriv) {
+					return core.ErrSpecificAccessDenied.GenWithStackByArgs("CREATE ROLE or CREATE USER")
+				}
 			}
 			if !s.IsCreateRole && !checker.RequestVerification(activeRoles, "", "", "", mysql.CreateUserPriv) {
 				return core.ErrSpecificAccessDenied.GenWithStackByArgs("CREATE User")
@@ -661,7 +664,10 @@ func (e *SimpleExec) executeCreateUser(ctx context.Context, s *ast.CreateUserStm
 		}
 		if exists {
 			if !s.IfNotExists {
-				return errors.New("Duplicate user")
+				if s.IsCreateRole {
+					return ErrCannotUser.GenWithStackByArgs("CREATE ROLE", spec.User.String())
+				}
+				return ErrCannotUser.GenWithStackByArgs("CREATE USER", spec.User.String())
 			}
 			continue
 		}
@@ -819,8 +825,11 @@ func (e *SimpleExec) executeDropUser(s *ast.DropUserStmt) error {
 		}
 		activeRoles := e.ctx.GetSessionVars().ActiveRoles
 		if !checker.RequestVerification(activeRoles, mysql.SystemDB, mysql.UserTable, "", mysql.DeletePriv) {
-			if s.IsDropRole && !checker.RequestVerification(activeRoles, "", "", "", mysql.DropRolePriv) {
-				return core.ErrSpecificAccessDenied.GenWithStackByArgs("DROP ROLE")
+			if s.IsDropRole {
+				if !checker.RequestVerification(activeRoles, "", "", "", mysql.DropRolePriv) &&
+					!checker.RequestVerification(activeRoles, "", "", "", mysql.CreateUserPriv) {
+					return core.ErrSpecificAccessDenied.GenWithStackByArgs("DROP ROLE or CREATE USER")
+				}
 			}
 			if !s.IsDropRole && !checker.RequestVerification(activeRoles, "", "", "", mysql.CreateUserPriv) {
 				return core.ErrSpecificAccessDenied.GenWithStackByArgs("CREATE USER")
diff --git a/executor/simple_test.go b/executor/simple_test.go
index c93a47453973c..02b49b5d3e192 100644
--- a/executor/simple_test.go
+++ b/executor/simple_test.go
@@ -58,6 +58,54 @@ func (s *testSuite3) TestDo(c *C) {
 	tk.MustQuery("select @a").Check(testkit.Rows("1"))
 }
 
+func (s *testSuite3) TestCreateRole(c *C) {
+	tk := testkit.NewTestKit(c, s.store)
+	tk.MustExec("create user testCreateRole;")
+	tk.MustExec("grant CREATE USER on *.* to testCreateRole;")
+	se, err := session.CreateSession4Test(s.store)
+	c.Check(err, IsNil)
+	defer se.Close()
+	c.Assert(se.Auth(&auth.UserIdentity{Username: "testCreateRole", Hostname: "localhost"}, nil, nil), IsTrue)
+
+	ctx := context.Background()
+	_, err = se.Execute(ctx, `create role test_create_role;`)
+	c.Assert(err, IsNil)
+	tk.MustExec("revoke CREATE USER on *.* from testCreateRole;")
+	tk.MustExec("drop role test_create_role;")
+	tk.MustExec("grant CREATE ROLE on *.* to testCreateRole;")
+	_, err = se.Execute(ctx, `create role test_create_role;`)
+	c.Assert(err, IsNil)
+	tk.MustExec("drop role test_create_role;")
+	_, err = se.Execute(ctx, `create user test_create_role;`)
+	c.Assert(err, NotNil)
+	tk.MustExec("drop user testCreateRole;")
+}
+
+func (s *testSuite3) TestDropRole(c *C) {
+	tk := testkit.NewTestKit(c, s.store)
+	tk.MustExec("create user testCreateRole;")
+	tk.MustExec("create user test_create_role;")
+	tk.MustExec("grant CREATE USER on *.* to testCreateRole;")
+	se, err := session.CreateSession4Test(s.store)
+	c.Check(err, IsNil)
+	defer se.Close()
+	c.Assert(se.Auth(&auth.UserIdentity{Username: "testCreateRole", Hostname: "localhost"}, nil, nil), IsTrue)
+
+	ctx := context.Background()
+	_, err = se.Execute(ctx, `drop role test_create_role;`)
+	c.Assert(err, IsNil)
+	tk.MustExec("revoke CREATE USER on *.* from testCreateRole;")
+	tk.MustExec("create role test_create_role;")
+	tk.MustExec("grant DROP ROLE on *.* to testCreateRole;")
+	_, err = se.Execute(ctx, `drop role test_create_role;`)
+	c.Assert(err, IsNil)
+	tk.MustExec("create user test_create_role;")
+	_, err = se.Execute(ctx, `drop user test_create_role;`)
+	c.Assert(err, NotNil)
+	tk.MustExec("drop user testCreateRole;")
+	tk.MustExec("drop user test_create_role;")
+}
+
 func (s *testSuite3) TestTransaction(c *C) {
 	tk := testkit.NewTestKit(c, s.store)
 	tk.MustExec("begin")