Skip to content

Commit

Permalink
privilege: fix atomic problem of DROP ROLE (pingcap#14090)
Browse files Browse the repository at this point in the history
  • Loading branch information
Lingyu Song committed Dec 19, 2019
1 parent c598e4b commit 5213da2
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 59 deletions.
89 changes: 34 additions & 55 deletions executor/simple.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -838,112 +838,91 @@ func (e *SimpleExec) executeDropUser(s *ast.DropUserStmt) error {
}

failedUsers := make([]string, 0, len(s.UserList))
notExistUsers := make([]string, 0, len(s.UserList))
sysSession, err := e.getSysSession()
defer e.releaseSysSession(sysSession)
if err != nil {
return err
}
sqlExecutor := sysSession.(sqlexec.SQLExecutor)

if _, err := sqlExecutor.Execute(context.Background(), "begin"); err != nil {
return err
}

for _, user := range s.UserList {
exists, err := userExists(e.ctx, user.Username, user.Hostname)
if err != nil {
return err
}
if !exists {
notExistUsers = append(notExistUsers, user.String())
continue
if s.IfExists {
e.ctx.GetSessionVars().StmtCtx.AppendNote(infoschema.ErrUserDropExists.GenWithStackByArgs(user))
} else {
failedUsers = append(failedUsers, user.String())
break
}
}

// begin a transaction to delete a user.
if _, err := sqlExecutor.Execute(context.Background(), "begin"); err != nil {
return err
}
sql := fmt.Sprintf(`DELETE FROM %s.%s WHERE Host = '%s' and User = '%s';`, mysql.SystemDB, mysql.UserTable, user.Hostname, user.Username)
if _, err := sqlExecutor.Execute(context.Background(), sql); err != nil {
if _, err = sqlExecutor.Execute(context.Background(), sql); err != nil {
failedUsers = append(failedUsers, user.String())
if _, err := sqlExecutor.Execute(context.Background(), "rollback"); err != nil {
return err
}
continue
break
}

// delete privileges from mysql.db
sql = fmt.Sprintf(`DELETE FROM %s.%s WHERE Host = '%s' and User = '%s';`, mysql.SystemDB, mysql.DBTable, user.Hostname, user.Username)
if _, err := sqlExecutor.Execute(context.Background(), sql); err != nil {
if _, err = sqlExecutor.Execute(context.Background(), sql); err != nil {
failedUsers = append(failedUsers, user.String())
if _, err := sqlExecutor.Execute(context.Background(), "rollback"); err != nil {
return err
}
continue
break
}

// delete privileges from mysql.tables_priv
sql = fmt.Sprintf(`DELETE FROM %s.%s WHERE Host = '%s' and User = '%s';`, mysql.SystemDB, mysql.TablePrivTable, user.Hostname, user.Username)
if _, err := sqlExecutor.Execute(context.Background(), sql); err != nil {
if _, err = sqlExecutor.Execute(context.Background(), sql); err != nil {
failedUsers = append(failedUsers, user.String())
if _, err := sqlExecutor.Execute(context.Background(), "rollback"); err != nil {
return err
}
continue
break
}

// delete relationship from mysql.role_edges
sql = fmt.Sprintf(`DELETE FROM %s.%s WHERE TO_HOST = '%s' and TO_USER = '%s';`, mysql.SystemDB, mysql.RoleEdgeTable, user.Hostname, user.Username)
if _, err := sqlExecutor.Execute(context.Background(), sql); err != nil {
if _, err = sqlExecutor.Execute(context.Background(), sql); err != nil {
failedUsers = append(failedUsers, user.String())
if _, err := sqlExecutor.Execute(context.Background(), "rollback"); err != nil {
return err
}
continue
break
}

sql = fmt.Sprintf(`DELETE FROM %s.%s WHERE FROM_HOST = '%s' and FROM_USER = '%s';`, mysql.SystemDB, mysql.RoleEdgeTable, user.Hostname, user.Username)
if _, err := sqlExecutor.Execute(context.Background(), sql); err != nil {
if _, err = sqlExecutor.Execute(context.Background(), sql); err != nil {
failedUsers = append(failedUsers, user.String())
if _, err := sqlExecutor.Execute(context.Background(), "rollback"); err != nil {
return err
}
continue
break
}

// delete relationship from mysql.default_roles
sql = fmt.Sprintf(`DELETE FROM %s.%s WHERE DEFAULT_ROLE_HOST = '%s' and DEFAULT_ROLE_USER = '%s';`, mysql.SystemDB, mysql.DefaultRoleTable, user.Hostname, user.Username)
if _, err := sqlExecutor.Execute(context.Background(), sql); err != nil {
if _, err = sqlExecutor.Execute(context.Background(), sql); err != nil {
failedUsers = append(failedUsers, user.String())
if _, err := sqlExecutor.Execute(context.Background(), "rollback"); err != nil {
return err
}
continue
break
}

sql = fmt.Sprintf(`DELETE FROM %s.%s WHERE HOST = '%s' and USER = '%s';`, mysql.SystemDB, mysql.DefaultRoleTable, user.Hostname, user.Username)
if _, err := sqlExecutor.Execute(context.Background(), sql); err != nil {
if _, err = sqlExecutor.Execute(context.Background(), sql); err != nil {
failedUsers = append(failedUsers, user.String())
if _, err := sqlExecutor.Execute(context.Background(), "rollback"); err != nil {
return err
}
continue
break
}

//TODO: need delete columns_priv once we implement columns_priv functionality.
if _, err := sqlExecutor.Execute(context.Background(), "commit"); err != nil {
failedUsers = append(failedUsers, user.String())
}
}

if len(notExistUsers) > 0 {
if s.IfExists {
for _, user := range notExistUsers {
e.ctx.GetSessionVars().StmtCtx.AppendNote(infoschema.ErrUserDropExists.GenWithStackByArgs(user))
}
} else {
failedUsers = append(failedUsers, notExistUsers...)
if len(failedUsers) == 0 {
if _, err := sqlExecutor.Execute(context.Background(), "commit"); err != nil {
return err
}
} else {
if _, err := sqlExecutor.Execute(context.Background(), "rollback"); err != nil {
return err
}
if s.IsDropRole {
return ErrCannotUser.GenWithStackByArgs("DROP ROLE", strings.Join(failedUsers, ","))
}
}

if len(failedUsers) > 0 {
return ErrCannotUser.GenWithStackByArgs("DROP USER", strings.Join(failedUsers, ","))
}
domain.GetDomain(e.ctx).NotifyUpdatePrivilege(e.ctx)
Expand Down
23 changes: 19 additions & 4 deletions executor/simple_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -377,11 +377,9 @@ func (s *testSuite3) TestUser(c *C) {
_, err = tk.Exec(dropUserSQL)
c.Check(err, NotNil)
dropUserSQL = `DROP USER 'test3'@'localhost';`
_, err = tk.Exec(dropUserSQL)
c.Check(err, NotNil)
tk.MustExec(dropUserSQL)
dropUserSQL = `DROP USER 'test1'@'localhost';`
_, err = tk.Exec(dropUserSQL)
c.Check(err, NotNil)
tk.MustExec(dropUserSQL)
// Test positive cases without IF EXISTS.
createUserSQL = `CREATE USER 'test1'@'localhost', 'test3'@'localhost';`
tk.MustExec(createUserSQL)
Expand Down Expand Up @@ -625,3 +623,20 @@ func (s *testSuite3) TestIssue9111(c *C) {

tk.MustExec("drop user 'user_admin'@'localhost';")
}

func (s *testSuite3) TestRoleAtomic(c *C) {
tk := testkit.NewTestKit(c, s.store)

tk.MustExec("create role r2;")
_, err := tk.Exec("create role r1, r2, r3")
c.Check(err, NotNil)
// Check atomic create role.
result := tk.MustQuery(`SELECT user FROM mysql.User WHERE user in ('r1', 'r2', 'r3')`)
result.Check(testkit.Rows("r2"))
// Check atomic drop role.
_, err = tk.Exec("drop role r1, r2, r3")
c.Check(err, NotNil)
result = tk.MustQuery(`SELECT user FROM mysql.User WHERE user in ('r1', 'r2', 'r3')`)
result.Check(testkit.Rows("r2"))
tk.MustExec("drop role r2;")
}

0 comments on commit 5213da2

Please sign in to comment.