From a77e4df40766c4ab5b948084f44407d8a246f2fd Mon Sep 17 00:00:00 2001 From: shenli Date: Fri, 30 Oct 2015 14:00:02 +0800 Subject: [PATCH 1/3] *: Add privilege check for drop table. --- ddl/ddl.go | 10 ++++++++++ privilege/privileges/privileges.go | 3 +++ privilege/privileges/privileges_test.go | 23 +++++++++++++++++++++++ 3 files changed, 36 insertions(+) diff --git a/ddl/ddl.go b/ddl/ddl.go index 92aab675c5..3ad5599603 100644 --- a/ddl/ddl.go +++ b/ddl/ddl.go @@ -31,6 +31,7 @@ import ( "github.com/pingcap/tidb/model" "github.com/pingcap/tidb/mysql" "github.com/pingcap/tidb/parser/coldef" + "github.com/pingcap/tidb/privilege" "github.com/pingcap/tidb/table" "github.com/pingcap/tidb/table/tables" "github.com/pingcap/tidb/util" @@ -576,6 +577,15 @@ func (d *ddl) DropTable(ctx context.Context, ti table.Ident) (err error) { if err != nil { return errors.Trace(err) } + // Check Privilege + pchecker := privilege.GetPrivilegeChecker(ctx) + hasPriv, err := pchecker.Check(ctx, schema, tb.Meta(), mysql.DropPriv) + if err != nil { + return errors.Trace(err) + } + if !hasPriv { + return errors.Errorf("You do not have the privilege to drop table %s.%s.", ti.Schema, ti.Name) + } err = kv.RunInNewTxn(d.store, false, func(txn kv.Transaction) error { t := meta.NewMeta(txn) diff --git a/privilege/privileges/privileges.go b/privilege/privileges/privileges.go index 499d7e9ac6..884901f09d 100644 --- a/privilege/privileges/privileges.go +++ b/privilege/privileges/privileges.go @@ -166,6 +166,9 @@ func (p *UserPrivileges) Check(ctx context.Context, db *model.DBInfo, tbl *model if len(p.User) == 0 { // User current user p.User = variable.GetSessionVars(ctx).User + if len(p.User) == 0 { + return true, nil + } } err := p.loadPrivileges(ctx) if err != nil { diff --git a/privilege/privileges/privileges_test.go b/privilege/privileges/privileges_test.go index 37c74ddca5..c383c63ac2 100644 --- a/privilege/privileges/privileges_test.go +++ b/privilege/privileges/privileges_test.go @@ -233,6 +233,29 @@ func (t *testPrivilegeSuite) TestShowGrants(c *C) { c.Assert(testutil.CompareUnorderedStringSlice(gs, expected), IsTrue) } +func (t *testPrivilegeSuite) TestDropTablePriv(c *C) { + se := newSession(c, t.store, t.dbName) + ctx, _ := se.(context.Context) + mustExec(c, se, `CREATE TABLE todrop(c int);`) + variable.GetSessionVars(ctx).User = "root@localhost" + mustExec(c, se, `CREATE USER 'drop'@'localhost' identified by '123';`) + mustExec(c, se, `GRANT Select ON test.todrop TO 'drop'@'localhost';`) + + variable.GetSessionVars(ctx).User = "drop@localhost" + mustExec(c, se, `SELECT * FROM todrop;`) + + _, err := se.Execute("DROP TABLE todrop;") + c.Assert(err, NotNil) + + variable.GetSessionVars(ctx).User = "root@localhost" + mustExec(c, se, `GRANT Drop ON test.todrop TO 'drop'@'localhost';`) + + se1 := newSession(c, t.store, t.dbName) + ctx1, _ := se.(context.Context) + variable.GetSessionVars(ctx1).User = "drop@localhost" + mustExec(c, se1, `DROP TABLE todrop;`) +} + func mustExec(c *C, se tidb.Session, sql string) { _, err := se.Execute(sql) c.Assert(err, IsNil) From f6e54fd7e92ef0072cb8c3047319f1096bbb0c83 Mon Sep 17 00:00:00 2001 From: Shen Li Date: Fri, 30 Oct 2015 19:19:31 +0800 Subject: [PATCH 2/3] *: Address comment --- ddl/ddl.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ddl/ddl.go b/ddl/ddl.go index 3ad5599603..c1e8002c3b 100644 --- a/ddl/ddl.go +++ b/ddl/ddl.go @@ -578,8 +578,8 @@ func (d *ddl) DropTable(ctx context.Context, ti table.Ident) (err error) { return errors.Trace(err) } // Check Privilege - pchecker := privilege.GetPrivilegeChecker(ctx) - hasPriv, err := pchecker.Check(ctx, schema, tb.Meta(), mysql.DropPriv) + privChecker := privilege.GetPrivilegeChecker(ctx) + hasPriv, err := privChecker.Check(ctx, schema, tb.Meta(), mysql.DropPriv) if err != nil { return errors.Trace(err) } From 1569edef98a564aafab688b5e13b731fded9166d Mon Sep 17 00:00:00 2001 From: Shen Li Date: Fri, 30 Oct 2015 23:07:36 +0800 Subject: [PATCH 3/3] privileges: Address comment --- privilege/privileges/privileges.go | 2 ++ privilege/privileges/privileges_test.go | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/privilege/privileges/privileges.go b/privilege/privileges/privileges.go index 884901f09d..e387e9ac70 100644 --- a/privilege/privileges/privileges.go +++ b/privilege/privileges/privileges.go @@ -167,6 +167,8 @@ func (p *UserPrivileges) Check(ctx context.Context, db *model.DBInfo, tbl *model // User current user p.User = variable.GetSessionVars(ctx).User if len(p.User) == 0 { + // In embedded db mode, user does not need to login. So we do not have username. + // TODO: remove this check latter. return true, nil } } diff --git a/privilege/privileges/privileges_test.go b/privilege/privileges/privileges_test.go index c383c63ac2..23d8ff0912 100644 --- a/privilege/privileges/privileges_test.go +++ b/privilege/privileges/privileges_test.go @@ -251,7 +251,7 @@ func (t *testPrivilegeSuite) TestDropTablePriv(c *C) { mustExec(c, se, `GRANT Drop ON test.todrop TO 'drop'@'localhost';`) se1 := newSession(c, t.store, t.dbName) - ctx1, _ := se.(context.Context) + ctx1, _ := se1.(context.Context) variable.GetSessionVars(ctx1).User = "drop@localhost" mustExec(c, se1, `DROP TABLE todrop;`) }