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.

Sign up for GitHub

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

Jump to bottom

executor: grant to non-existent table with at least create privilege #29273

Merged
merged 5 commits into from
Nov 1, 2021
Merged

executor: grant to non-existent table with at least create privilege #29273

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
96f6d43
executor: grant to non-existent table with at least create privilege
unconsolable Oct 30, 2021
47b305b
executor: remove redundant code in test
unconsolable Oct 31, 2021
01ccb8f
fix import
unconsolable Oct 31, 2021
f855173
executor: check whether privilege takes effect after creation
unconsolable Oct 31, 2021
5e3616e
Merge branch 'master' into issue-29268
ti-chi-bot Oct 31, 2021
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
17 changes: 14 additions & 3 deletions executor/grant.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,20 @@ func (e *GrantExec) Next(ctx context.Context, req *chunk.Chunk) error {
dbNameStr := model.NewCIStr(dbName)
schema := e.ctx.GetInfoSchema().(infoschema.InfoSchema)
tbl, err := schema.TableByName(dbNameStr, model.NewCIStr(e.Level.TableName))
// Allow GRANT on non-existent table, see issue #28533
if err != nil && !terror.ErrorEqual(err, infoschema.ErrTableNotExists) {
return err
// Allow GRANT on non-existent table with at least create privilege, see issue #28533 #29268
if err != nil {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Will this pr cause some compatibility problems?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

IMHO it won't as it is based on previous PR #28882, and both of them have not delivered to release branch. Or we can wait on @morgo's opinion.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with you @unconsolable. It will only cause compatibility issues between from an unreleased version.

allowed := false
if terror.ErrorEqual(err, infoschema.ErrTableNotExists) {
for _, p := range e.Privs {
if p.Priv == mysql.AllPriv || p.Priv&mysql.CreatePriv > 0 {
allowed = true
break
}
}
}
if !allowed {
return err
}
}
// Note the table name compare is case sensitive here.
if tbl != nil && tbl.Meta().Name.String() != e.Level.TableName {
Expand Down
42 changes: 40 additions & 2 deletions executor/grant_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -462,8 +462,8 @@ func TestGrantOnNonExistTable(t *testing.T) {
tk.MustExec("use test")
_, err := tk.Exec("select * from nonexist")
require.True(t, terror.ErrorEqual(err, infoschema.ErrTableNotExists))
// GRANT ON non-existent table success, see issue #28533
tk.MustExec("grant Select,Insert on nonexist to 'genius'")
_, err = tk.Exec("grant Select,Insert on nonexist to 'genius'")
require.True(t, terror.ErrorEqual(err, infoschema.ErrTableNotExists))

tk.MustExec("create table if not exists xx (id int)")
// Case sensitive
Expand All @@ -474,6 +474,44 @@ func TestGrantOnNonExistTable(t *testing.T) {
require.NoError(t, err)
_, err = tk.Exec("grant Select,Update on test.xx to 'genius'")
require.NoError(t, err)

// issue #29268
tk.MustExec("CREATE DATABASE d29268")
defer tk.MustExec("DROP DATABASE IF EXISTS d29268")
tk.MustExec("USE d29268")
tk.MustExec("CREATE USER u29268")
defer tk.MustExec("DROP USER u29268")

// without create privilege
err = tk.ExecToErr("GRANT SELECT ON t29268 TO u29268")
require.Error(t, err)
require.True(t, terror.ErrorEqual(err, infoschema.ErrTableNotExists))
err = tk.ExecToErr("GRANT DROP, INSERT ON t29268 TO u29268")
require.Error(t, err)
require.True(t, terror.ErrorEqual(err, infoschema.ErrTableNotExists))
err = tk.ExecToErr("GRANT UPDATE, CREATE VIEW, SHOW VIEW ON t29268 TO u29268")
require.Error(t, err)
require.True(t, terror.ErrorEqual(err, infoschema.ErrTableNotExists))
err = tk.ExecToErr("GRANT DELETE, REFERENCES, ALTER ON t29268 TO u29268")
require.Error(t, err)
require.True(t, terror.ErrorEqual(err, infoschema.ErrTableNotExists))

// with create privilege
tk.MustExec("GRANT CREATE ON t29268 TO u29268")
tk.MustExec("GRANT CREATE, SELECT ON t29268 TO u29268")
tk.MustExec("GRANT CREATE, DROP, INSERT ON t29268 TO u29268")

// check privilege
tk.Session().Auth(&auth.UserIdentity{Username: "u29268", Hostname: "localhost"}, nil, nil)
tk.MustExec("USE d29268")
tk.MustExec("CREATE TABLE t29268 (c1 int)")
tk.MustExec("INSERT INTO t29268 VALUES (1), (2)")
tk.MustQuery("SELECT c1 FROM t29268").Check(testkit.Rows("1", "2"))
tk.MustExec("DROP TABLE t29268")

// check grant all
tk.Session().Auth(&auth.UserIdentity{Username: "root", Hostname: "localhost"}, nil, nil)
tk.MustExec("GRANT ALL ON t29268 TO u29268")
}

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