privilege: fix privilege check of CREATE ROLE and DROP ROLE (ping…

…cap#13940)
imtbkcat · Dec 17, 2019 · 21e99e3 · 21e99e3
1 parent 5d5b071
commit 21e99e3
Show file tree

Hide file tree

Showing 2 changed files with 62 additions and 5 deletions.
diff --git 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
@@ -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")