From 88c105d0b90b71714ae5457e6d7e70a18bc20fe1 Mon Sep 17 00:00:00 2001 From: Morgan Tocker Date: Fri, 2 Nov 2018 10:59:47 -0600 Subject: [PATCH 1/4] executor: support NO_AUTO_CREATE_USER --- executor/grant.go | 5 ++++- executor/grant_test.go | 12 ++++++++++++ executor/simple_test.go | 1 - session/session_test.go | 5 +++-- 4 files changed, 19 insertions(+), 4 deletions(-) diff --git a/executor/grant.go b/executor/grant.go index 4225043e21fd0..6bd52b927e259 100644 --- a/executor/grant.go +++ b/executor/grant.go @@ -70,7 +70,10 @@ func (e *GrantExec) Next(ctx context.Context, chk *chunk.Chunk) error { if err != nil { return errors.Trace(err) } - if !exists { + // TODO: Change to SQLMode.HasAuthCreateUserMode() once parser supports it. + if !exists && e.ctx.GetSessionVars().SQLMode.HasStrictMode() { + return fmt.Errorf("Can't find any matching row in the user table") + } else if !exists { pwd, ok := user.EncodedPassword() if !ok { return errors.Trace(ErrPasswordFormat) diff --git a/executor/grant_test.go b/executor/grant_test.go index bb3d33262ebea..3700bb17dac08 100644 --- a/executor/grant_test.go +++ b/executor/grant_test.go @@ -181,14 +181,26 @@ func (s *testSuite) TestIssue2456(c *C) { tk.MustExec("GRANT ALL PRIVILEGES ON `dddb_%`.`te%` to 'dduser'@'%';") } +func (s *testSuite) TestNoAutoCreateUser(c *C) { + // Creating a user as part of a GRANT is no longer the default + // sql_mode = NO_AUTO_CREATE_USER + tk := testkit.NewTestKit(c, s.store) + tk.MustExec(`DROP USER IF EXISTS 'test'@'%'`) + _, err := tk.Exec(`GRANT ALL PRIVILEGES ON *.* to 'test'@'%' IDENTIFIED BY 'xxx'`) + c.Check(err, NotNil) +} + func (s *testSuite) TestCreateUserWhenGrant(c *C) { tk := testkit.NewTestKit(c, s.store) tk.MustExec(`DROP USER IF EXISTS 'test'@'%'`) + // This only applies to sql_mode:NO_AUTO_CREATE_USER off + tk.MustExec(`SET SQL_MODE=''`) tk.MustExec(`GRANT ALL PRIVILEGES ON *.* to 'test'@'%' IDENTIFIED BY 'xxx'`) // Make sure user is created automatically when grant to a non-exists one. tk.MustQuery(`SELECT user FROM mysql.user WHERE user='test' and host='%'`).Check( testkit.Rows("test"), ) + tk.MustExec(`DROP USER IF EXISTS 'test'@'%'`) } func (s *testSuite) TestIssue2654(c *C) { diff --git a/executor/simple_test.go b/executor/simple_test.go index 0564f01bc6f87..ffd819d98d845 100644 --- a/executor/simple_test.go +++ b/executor/simple_test.go @@ -181,7 +181,6 @@ func (s *testSuite) TestUser(c *C) { "localhost test testDB Y Y Y Y Y Y Y N Y Y N N N N N N Y N N", "localhost test testDB1 Y Y Y Y Y Y Y N Y Y N N N N N N Y N N", "% dddb_% dduser Y Y Y Y Y Y Y N Y Y N N N N N N Y N N", - "% test test Y N N N N N N N N N N N N N N N N N N", "localhost test testDBRevoke N N N N N N N N N N N N N N N N N N N", )) diff --git a/session/session_test.go b/session/session_test.go index 57aa93e0477c4..76b9b12272d83 100644 --- a/session/session_test.go +++ b/session/session_test.go @@ -2034,8 +2034,9 @@ func (s *testSessionSuite) TestDBUserNameLength(c *C) { tk := testkit.NewTestKitWithInit(c, s.store) tk.MustExec("create table if not exists t (a int)") // Test user name length can be longer than 16. - tk.MustExec(`grant all privileges on test.* to 'abcddfjakldfjaldddds'@'%' identified by ''`) - tk.MustExec(`grant all privileges on test.t to 'abcddfjakldfjaldddds'@'%' identified by ''`) + tk.MustExec(`CREATE USER 'abcddfjakldfjaldddds'@'%' identified by ''`) + tk.MustExec(`grant all privileges on test.* to 'abcddfjakldfjaldddds'@'%'`) + tk.MustExec(`grant all privileges on test.t to 'abcddfjakldfjaldddds'@'%'`) } func (s *testSessionSuite) TestKVVars(c *C) { From 90ea95acbc38242d4a1b7a66c0781e9b8fe19b5b Mon Sep 17 00:00:00 2001 From: Morgan Tocker Date: Fri, 2 Nov 2018 13:28:05 -0600 Subject: [PATCH 2/4] executor: error based on correct sql mode. --- executor/grant.go | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/executor/grant.go b/executor/grant.go index 6bd52b927e259..ba652d35fd69c 100644 --- a/executor/grant.go +++ b/executor/grant.go @@ -21,6 +21,7 @@ import ( "github.com/pingcap/parser/ast" "github.com/pingcap/parser/model" "github.com/pingcap/parser/mysql" + "github.com/pingcap/parser/terror" "github.com/pingcap/tidb/domain" "github.com/pingcap/tidb/infoschema" "github.com/pingcap/tidb/sessionctx" @@ -38,6 +39,14 @@ var ( _ Executor = (*GrantExec)(nil) ) +const ( + codePasswordNoMatch = 1133 +) + +var ( + errPasswordNoMatch = terror.ClassExecutor.New(codePasswordNoMatch, mysql.MySQLErrName[mysql.ErrPasswordNoMatch]) +) + // GrantExec executes GrantStmt. type GrantExec struct { baseExecutor @@ -70,9 +79,8 @@ func (e *GrantExec) Next(ctx context.Context, chk *chunk.Chunk) error { if err != nil { return errors.Trace(err) } - // TODO: Change to SQLMode.HasAuthCreateUserMode() once parser supports it. - if !exists && e.ctx.GetSessionVars().SQLMode.HasStrictMode() { - return fmt.Errorf("Can't find any matching row in the user table") + if !exists && e.ctx.GetSessionVars().SQLMode.HasNoAutoCreateUserMode() { + return errPasswordNoMatch } else if !exists { pwd, ok := user.EncodedPassword() if !ok { From 142c8c4b61f07e4e211904dacd834bd6e9a552de Mon Sep 17 00:00:00 2001 From: Morgan Tocker Date: Fri, 2 Nov 2018 13:56:49 -0600 Subject: [PATCH 3/4] Make test work with sql-mode not enabled --- executor/grant_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/executor/grant_test.go b/executor/grant_test.go index 3700bb17dac08..9f615db62e308 100644 --- a/executor/grant_test.go +++ b/executor/grant_test.go @@ -182,10 +182,9 @@ func (s *testSuite) TestIssue2456(c *C) { } func (s *testSuite) TestNoAutoCreateUser(c *C) { - // Creating a user as part of a GRANT is no longer the default - // sql_mode = NO_AUTO_CREATE_USER tk := testkit.NewTestKit(c, s.store) tk.MustExec(`DROP USER IF EXISTS 'test'@'%'`) + tk.MustExec(`SET sql_mode='NO_AUTO_CREATE_USER'`) _, err := tk.Exec(`GRANT ALL PRIVILEGES ON *.* to 'test'@'%' IDENTIFIED BY 'xxx'`) c.Check(err, NotNil) } From 9386120591fac507edf54a6c62f9a6592dd45c98 Mon Sep 17 00:00:00 2001 From: Morgan Tocker Date: Mon, 5 Nov 2018 23:13:44 +0000 Subject: [PATCH 4/4] Addressed PR feedback --- executor/grant.go | 11 +---------- executor/grant_test.go | 3 +++ 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/executor/grant.go b/executor/grant.go index ba652d35fd69c..e8fe3b4291140 100644 --- a/executor/grant.go +++ b/executor/grant.go @@ -21,7 +21,6 @@ import ( "github.com/pingcap/parser/ast" "github.com/pingcap/parser/model" "github.com/pingcap/parser/mysql" - "github.com/pingcap/parser/terror" "github.com/pingcap/tidb/domain" "github.com/pingcap/tidb/infoschema" "github.com/pingcap/tidb/sessionctx" @@ -39,14 +38,6 @@ var ( _ Executor = (*GrantExec)(nil) ) -const ( - codePasswordNoMatch = 1133 -) - -var ( - errPasswordNoMatch = terror.ClassExecutor.New(codePasswordNoMatch, mysql.MySQLErrName[mysql.ErrPasswordNoMatch]) -) - // GrantExec executes GrantStmt. type GrantExec struct { baseExecutor @@ -80,7 +71,7 @@ func (e *GrantExec) Next(ctx context.Context, chk *chunk.Chunk) error { return errors.Trace(err) } if !exists && e.ctx.GetSessionVars().SQLMode.HasNoAutoCreateUserMode() { - return errPasswordNoMatch + return ErrPasswordNoMatch } else if !exists { pwd, ok := user.EncodedPassword() if !ok { diff --git a/executor/grant_test.go b/executor/grant_test.go index 9f615db62e308..eff26c2e60680 100644 --- a/executor/grant_test.go +++ b/executor/grant_test.go @@ -19,6 +19,8 @@ import ( . "github.com/pingcap/check" "github.com/pingcap/parser/mysql" + "github.com/pingcap/parser/terror" + "github.com/pingcap/tidb/executor" "github.com/pingcap/tidb/util/testkit" ) @@ -187,6 +189,7 @@ func (s *testSuite) TestNoAutoCreateUser(c *C) { tk.MustExec(`SET sql_mode='NO_AUTO_CREATE_USER'`) _, err := tk.Exec(`GRANT ALL PRIVILEGES ON *.* to 'test'@'%' IDENTIFIED BY 'xxx'`) c.Check(err, NotNil) + c.Assert(terror.ErrorEqual(err, executor.ErrPasswordNoMatch), IsTrue) } func (s *testSuite) TestCreateUserWhenGrant(c *C) {