Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

executor: fix revoke result view bug #38552

Merged
merged 16 commits into from
Oct 28, 2022
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 32 additions & 13 deletions executor/revoke.go
Original file line number Diff line number Diff line change
Expand Up @@ -271,13 +271,22 @@ func (e *RevokeExec) revokeTablePriv(internalSession sessionctx.Context, priv *a
}
sql := new(strings.Builder)
sqlexec.MustFormatSQL(sql, "UPDATE %n.%n SET ", mysql.SystemDB, mysql.TablePrivTable)
err = composeTablePrivUpdateForRevoke(internalSession, sql, priv.Priv, user, host, dbName, tblName)
isDelRow, err := composeTablePrivUpdateForRevoke(internalSession, sql, priv.Priv, user, host, dbName, tblName)
if err != nil {
return err
}
sqlexec.MustFormatSQL(sql, " WHERE User=%? AND Host=%? AND DB=%? AND Table_name=%?", user, host, dbName, tblName)

sqlexec.MustFormatSQL(sql, " WHERE User=%? AND Host=%? AND DB=%? AND Table_name=%?", user, host, dbName, tblName)
_, err = internalSession.(sqlexec.SQLExecutor).ExecuteInternal(ctx, sql.String())
if err != nil {
return err
}

if isDelRow {
sql.Reset()
sqlexec.MustFormatSQL(sql, "DELETE FROM %n.%n WHERE User=%? AND Host=%? AND DB=%? AND Table_name=%?", mysql.SystemDB, mysql.TablePrivTable, user, host, dbName, tblName)
_, err = internalSession.(sqlexec.SQLExecutor).ExecuteInternal(ctx, sql.String())
}
return err
}

Expand All @@ -296,7 +305,7 @@ func (e *RevokeExec) revokeColumnPriv(internalSession sessionctx.Context, priv *

sql.Reset()
sqlexec.MustFormatSQL(sql, "UPDATE %n.%n SET ", mysql.SystemDB, mysql.ColumnPrivTable)
err = composeColumnPrivUpdateForRevoke(internalSession, sql, priv.Priv, user, host, dbName, tbl.Meta().Name.O, col.Name.O)
isDelRow, err := composeColumnPrivUpdateForRevoke(internalSession, sql, priv.Priv, user, host, dbName, tbl.Meta().Name.O, col.Name.O)
if err != nil {
return err
}
Expand All @@ -306,6 +315,17 @@ func (e *RevokeExec) revokeColumnPriv(internalSession sessionctx.Context, priv *
if err != nil {
return err
}

if isDelRow {
sql.Reset()
sqlexec.MustFormatSQL(sql, "DELETE FROM %n.%n WHERE User=%? AND Host=%? AND DB=%? AND Table_name=%? AND Column_name=%?", mysql.SystemDB, mysql.ColumnPrivTable, user, host, dbName, tbl.Meta().Name.O, col.Name.O)
_, err = internalSession.(sqlexec.SQLExecutor).ExecuteInternal(ctx, sql.String())
if err != nil {
return err
}
break
}
//TODO Optimized for batch, one-shot.
}
return nil
}
Expand All @@ -319,12 +339,12 @@ func privUpdateForRevoke(cur []string, priv mysql.PrivilegeType) ([]string, erro
return cur, nil
}

func composeTablePrivUpdateForRevoke(ctx sessionctx.Context, sql *strings.Builder, priv mysql.PrivilegeType, name string, host string, db string, tbl string) error {
func composeTablePrivUpdateForRevoke(ctx sessionctx.Context, sql *strings.Builder, priv mysql.PrivilegeType, name string, host string, db string, tbl string) (bool, error) {
var newTablePriv, newColumnPriv []string

currTablePriv, currColumnPriv, err := getTablePriv(ctx, name, host, db, tbl)
if err != nil {
return err
return false, err
}

if priv == mysql.AllPriv {
Expand All @@ -340,36 +360,35 @@ func composeTablePrivUpdateForRevoke(ctx sessionctx.Context, sql *strings.Builde
newTablePriv = SetFromString(currTablePriv)
newTablePriv, err = privUpdateForRevoke(newTablePriv, priv)
if err != nil {
return err
return false, err
}

newColumnPriv = SetFromString(currColumnPriv)
newColumnPriv, err = privUpdateForRevoke(newColumnPriv, priv)
if err != nil {
return err
return false, err
}
}

sqlexec.MustFormatSQL(sql, `Table_priv=%?, Column_priv=%?, Grantor=%?`, strings.Join(newTablePriv, ","), strings.Join(newColumnPriv, ","), ctx.GetSessionVars().User.String())
return nil
return len(newTablePriv) == 0, nil
}

func composeColumnPrivUpdateForRevoke(ctx sessionctx.Context, sql *strings.Builder, priv mysql.PrivilegeType, name string, host string, db string, tbl string, col string) error {
func composeColumnPrivUpdateForRevoke(ctx sessionctx.Context, sql *strings.Builder, priv mysql.PrivilegeType, name string, host string, db string, tbl string, col string) (bool, error) {
var newColumnPriv []string

if priv != mysql.AllPriv {
currColumnPriv, err := getColumnPriv(ctx, name, host, db, tbl, col)
if err != nil {
return err
return false, err
}

newColumnPriv = SetFromString(currColumnPriv)
newColumnPriv, err = privUpdateForRevoke(newColumnPriv, priv)
if err != nil {
return err
return false, err
}
}

sqlexec.MustFormatSQL(sql, `Column_priv=%?`, strings.Join(newColumnPriv, ","))
return nil
return len(newColumnPriv) == 0, nil
}
72 changes: 57 additions & 15 deletions executor/revoke_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,28 +98,35 @@ func TestRevokeTableScope(t *testing.T) {
res.Check(testkit.Rows("Select,Insert,Update,Delete,Create,Drop,Index,Alter,Create View,Show View,Trigger,References"))

// Revoke each priv from the user.
for _, v := range mysql.AllTablePrivs {
for index, v := range mysql.AllTablePrivs {
sql := fmt.Sprintf("REVOKE %s ON test.test1 FROM 'testTblRevoke'@'localhost';", mysql.Priv2Str[v])
tk.MustExec(sql)
rows := tk.MustQuery(`SELECT Table_priv FROM mysql.tables_priv WHERE User="testTblRevoke" and host="localhost" and db="test" and Table_name="test1";`).Rows()
require.Len(t, rows, 1)
row := rows[0]
require.Len(t, row, 1)

op := v.SetString()
found := false
for _, p := range executor.SetFromString(fmt.Sprintf("%s", row[0])) {
if op == p {
found = true
break
if index < len(mysql.AllTablePrivs)-1 {
require.Len(t, rows, 1)
row := rows[0]
require.Len(t, row, 1)
op := v.SetString()
found := false
for _, p := range executor.SetFromString(fmt.Sprintf("%s", row[0])) {
if op == p {
found = true
break
}
}
require.False(t, found, "%s", mysql.Priv2SetStr[v])
} else {
//delete row when last prv , updated by issue #38421
require.Len(t, rows, 0)
}
require.False(t, found, "%s", mysql.Priv2SetStr[v])
}

// Revoke all table scope privs.
tk.MustExec(`GRANT ALL PRIVILEGES ON test.test1 TO 'testTblRevoke'@'localhost';`)
tk.MustExec("REVOKE ALL ON test.test1 FROM 'testTblRevoke'@'localhost';")
tk.MustQuery(`SELECT Table_priv FROM mysql.Tables_priv WHERE User="testTblRevoke" and host="localhost" and db="test" and Table_name="test1"`).Check(testkit.Rows(""))
//delete row when last prv , updated by issue #38421
rows := tk.MustQuery(`SELECT Table_priv FROM mysql.Tables_priv WHERE User="testTblRevoke" and host="localhost" and db="test" and Table_name="test1"`).Rows()
require.Len(t, rows, 0)
}

func TestRevokeColumnScope(t *testing.T) {
Expand All @@ -145,7 +152,9 @@ func TestRevokeColumnScope(t *testing.T) {
require.Greater(t, strings.Index(p, mysql.Priv2SetStr[v]), -1)

tk.MustExec(revokeSQL)
tk.MustQuery(checkSQL).Check(testkit.Rows(""))
//delete row when last prv , updated by issue #38421
rows = tk.MustQuery(checkSQL).Rows()
require.Len(t, rows, 0)
}

// Create a new user.
Expand All @@ -163,7 +172,40 @@ func TestRevokeColumnScope(t *testing.T) {
require.Greater(t, strings.Index(p, mysql.Priv2SetStr[v]), -1)
}
tk.MustExec("REVOKE ALL(c2) ON test3 FROM 'testCol1Revoke'@'localhost'")
tk.MustQuery(`SELECT Column_priv FROM mysql.Columns_priv WHERE User="testCol1Revoke" and host="localhost" and db="test" and Table_name="test3"`).Check(testkit.Rows(""))
//delete row when last prv , updated by issue #38421
rows := tk.MustQuery(`SELECT Column_priv FROM mysql.Columns_priv WHERE User="testCol1Revoke" and host="localhost" and db="test" and Table_name="test3"`).Rows()
require.Len(t, rows, 0)
}

// ref issue #38421
func TestRevokeTableSingle(t *testing.T) {
wxbty marked this conversation as resolved.
Show resolved Hide resolved
store := testkit.CreateMockStore(t)
tk := testkit.NewTestKit(t, store)
// Create a new user.
tk.MustExec(`CREATE USER test;`)
tk.MustExec(`CREATE TABLE test.test1(c1 int);`)
tk.MustExec(`GRANT SELECT ON test.test1 TO test;`)

tk.MustExec(`REVOKE SELECT ON test.test1 from test;`)

rows := tk.MustQuery(`SELECT Column_priv FROM mysql.tables_priv WHERE User="test" `).Rows()
require.Len(t, rows, 0)
}

// ref issue #38421(column fix)
func TestRevokeTableSingleColumn(t *testing.T) {
store := testkit.CreateMockStore(t)
tk := testkit.NewTestKit(t, store)
// Create a new user.
tk.MustExec(`CREATE USER test;`)
tk.MustExec(`GRANT SELECT(Host) ON mysql.db TO test`)
tk.MustExec(`GRANT SELECT(DB) ON mysql.db TO test`)
tk.MustExec(`REVOKE SELECT(Host) ON mysql.db FROM test`)

rows := tk.MustQuery(`SELECT Column_priv FROM mysql.columns_priv WHERE User="test" and Column_name ='Host' `).Rows()
require.Len(t, rows, 0)
rows = tk.MustQuery(`SELECT Column_priv FROM mysql.columns_priv WHERE User="test" and Column_name ='DB' `).Rows()
require.Len(t, rows, 1)
}

func TestRevokeDynamicPrivs(t *testing.T) {
Expand Down