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: grant table level privilege should check table exist (#14540) #14611

Merged
merged 5 commits into from Feb 12, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
23 changes: 23 additions & 0 deletions executor/grant.go
Expand Up @@ -67,6 +67,29 @@ func (e *GrantExec) Next(ctx context.Context, req *chunk.Chunk) error {
if len(dbName) == 0 {
dbName = e.ctx.GetSessionVars().CurrentDB
}

// Make sure the table exist.
if e.Level.Level == ast.GrantLevelTable {
dbNameStr := model.NewCIStr(dbName)
schema := infoschema.GetInfoSchema(e.ctx)
tbl, err := schema.TableByName(dbNameStr, model.NewCIStr(e.Level.TableName))
if err != nil {
return err
}
err = infoschema.ErrTableNotExists.GenWithStackByArgs(dbName, e.Level.TableName)
// Note the table name compare is case sensitive here.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tiancaiamao Hi, may i ask. The TiDB lower_case_table_names is default 2. So in this, i think we need check case insensitive here? Some case in #34610

if tbl.Meta().Name.String() != e.Level.TableName {
return err
}
if len(e.Level.DBName) > 0 {
// The database name should also match.
db, succ := schema.SchemaByName(dbNameStr)
if !succ || db.Name.String() != dbName {
return err
}
}
}

// Grant for each user
for idx, user := range e.Users {
// Check if user exists.
Expand Down
26 changes: 26 additions & 0 deletions executor/grant_test.go
Expand Up @@ -21,6 +21,7 @@ import (
"github.com/pingcap/parser/mysql"
"github.com/pingcap/parser/terror"
"github.com/pingcap/tidb/executor"
"github.com/pingcap/tidb/infoschema"
"github.com/pingcap/tidb/util/testkit"
)

Expand Down Expand Up @@ -192,6 +193,8 @@ func (s *testSuite3) TestColumnScope(c *C) {
func (s *testSuite3) TestIssue2456(c *C) {
tk := testkit.NewTestKit(c, s.store)
tk.MustExec("CREATE USER 'dduser'@'%' IDENTIFIED by '123456';")
tk.MustExec("CREATE DATABASE `dddb_%`;")
tk.MustExec("CREATE table `dddb_%`.`te%` (id int);")
tk.MustExec("GRANT ALL PRIVILEGES ON `dddb_%`.* TO 'dduser'@'%';")
tk.MustExec("GRANT ALL PRIVILEGES ON `dddb_%`.`te%` to 'dduser'@'%';")
}
Expand Down Expand Up @@ -307,3 +310,26 @@ func (s *testSuite3) TestMaintainRequire(c *C) {
_, err = tk.Exec(`CREATE USER 'u9'@'%' require x509 x509`)
c.Assert(err, NotNil)
}

func (s *testSuite3) TestGrantOnNonExistTable(c *C) {
tk := testkit.NewTestKit(c, s.store)
tk.MustExec("create user genius")
tk.MustExec("use test")
_, err := tk.Exec("select * from nonexist")
c.Assert(terror.ErrorEqual(err, infoschema.ErrTableNotExists), IsTrue)
_, err = tk.Exec("grant Select,Insert on nonexist to 'genius'")
c.Assert(terror.ErrorEqual(err, infoschema.ErrTableNotExists), IsTrue)

tk.MustExec("create table if not exists xx (id int)")
// Case sensitive
_, err = tk.Exec("grant Select,Insert on XX to 'genius'")
c.Assert(terror.ErrorEqual(err, infoschema.ErrTableNotExists), IsTrue)
// The database name should also case sensitive match.
_, err = tk.Exec("grant Select,Insert on Test.xx to 'genius'")
c.Assert(terror.ErrorEqual(err, infoschema.ErrTableNotExists), IsTrue)

_, err = tk.Exec("grant Select,Insert on xx to 'genius'")
c.Assert(err, IsNil)
_, err = tk.Exec("grant Select,Update on test.xx to 'genius'")
c.Assert(err, IsNil)
}
2 changes: 1 addition & 1 deletion executor/simple_test.go
Expand Up @@ -607,7 +607,7 @@ func (s *testSuite3) TestIssue9111(c *C) {
c.Check(err, IsNil)

tk.MustExec("revoke create user on *.* from 'user_admin'@'localhost';")
tk.MustExec("grant insert, delete on mysql.User to 'user_admin'@'localhost';")
tk.MustExec("grant insert, delete on mysql.user to 'user_admin'@'localhost';")

_, err = se.Execute(ctx, `flush privileges`)
c.Check(err, IsNil)
Expand Down
18 changes: 18 additions & 0 deletions infoschema/infoschema.go
Expand Up @@ -14,6 +14,7 @@
package infoschema

import (
"context"
"sort"
"sync/atomic"

Expand All @@ -22,7 +23,10 @@ import (
"github.com/pingcap/parser/terror"
"github.com/pingcap/tidb/kv"
"github.com/pingcap/tidb/meta/autoid"
"github.com/pingcap/tidb/sessionctx"
"github.com/pingcap/tidb/table"
"github.com/pingcap/tidb/util/logutil"
"go.uber.org/zap"
)

var (
Expand Down Expand Up @@ -398,3 +402,17 @@ func HasAutoIncrementColumn(tbInfo *model.TableInfo) (bool, string) {
}
return false, ""
}

// GetInfoSchema gets TxnCtx InfoSchema if snapshot schema is not set,
// Otherwise, snapshot schema is returned.
func GetInfoSchema(ctx sessionctx.Context) InfoSchema {
sessVar := ctx.GetSessionVars()
var is InfoSchema
if snap := sessVar.SnapshotInfoschema; snap != nil {
is = snap.(InfoSchema)
logutil.Logger(context.Background()).Info("use snapshot schema", zap.Uint64("conn", sessVar.ConnectionID), zap.Int64("schemaVersion", is.SchemaMetaVersion()))
} else {
is = sessVar.TxnCtx.InfoSchema.(InfoSchema)
}
return is
}
1 change: 1 addition & 0 deletions planner/core/integration_test.go
Expand Up @@ -261,6 +261,7 @@ func (s *testIntegrationSuite) TestErrNoDB(c *C) {
_, err := tk.Exec("grant select on test1111 to test@'%'")
c.Assert(errors.Cause(err), Equals, core.ErrNoDB)
tk.MustExec("use test")
tk.MustExec("create table test1111 (id int)")
tk.MustExec("grant select on test1111 to test@'%'")
}

Expand Down
3 changes: 2 additions & 1 deletion privilege/privileges/privileges_test.go
Expand Up @@ -362,7 +362,8 @@ func (s *testPrivilegeSuite) TestShowGrants(c *C) {
c.Assert(err, IsNil)
c.Assert(gs, HasLen, 3)
mustExec(c, se, `GRANT INSERT, DELETE ON test.test TO 'r2'`)
mustExec(c, se, `GRANT UPDATE ON a.b TO 'testrole'@'localhost'`)
mustExec(c, se, `create table test.b (id int)`)
mustExec(c, se, `GRANT UPDATE ON test.b TO 'testrole'@'localhost'`)
gs, err = pc.ShowGrants(se, &auth.UserIdentity{Username: "testrole", Hostname: "localhost"}, roles)
c.Assert(err, IsNil)
c.Assert(gs, HasLen, 5)
Expand Down
4 changes: 3 additions & 1 deletion session/session_test.go
Expand Up @@ -2750,8 +2750,10 @@ func (s *testSessionSuite) TestGrantViewRelated(c *C) {
err = tkUser.ExecToErr("create view v_version29_c as select * from v_version29;")
c.Assert(err, NotNil)

tkRoot.MustExec(`grant create view on v_version29_c to 'u_version29'@'%'`)
tkRoot.MustExec("create view v_version29_c as select * from v_version29;")
tkRoot.MustExec(`grant create view on v_version29_c to 'u_version29'@'%'`) // Can't grant privilege on a non-exist table/view.
tkRoot.MustQuery("select table_priv from mysql.tables_priv where host='%' and db='test' and user='u_version29' and table_name='v_version29_c'").Check(testkit.Rows("Create View"))
tkRoot.MustExec("drop view v_version29_c")

tkRoot.MustExec(`grant select on v_version29 to 'u_version29'@'%'`)
tkUser.MustQuery("select current_user();").Check(testkit.Rows("u_version29@%"))
Expand Down