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

privilege,executor: update DBIsVisible() function for RBAC #10261

Merged
merged 3 commits into from
May 8, 2019
Merged

privilege,executor: update DBIsVisible() function for RBAC #10261

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f1363e4
privilege,executor: update DBIsVisible() function for RBAC
tiancaiamao Apr 25, 2019
9d2a773
address comment
tiancaiamao May 8, 2019
d254e83
Merge branch 'master' into db-visible-role
May 8, 2019
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
20 changes: 13 additions & 7 deletions executor/show.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ func (e *ShowExec) fetchShowDatabases() error {
// let information_schema be the first database
moveInfoSchemaToFront(dbs)
for _, d := range dbs {
if checker != nil && !checker.DBIsVisible(d) {
if checker != nil && !checker.DBIsVisible(e.ctx.GetSessionVars().ActiveRoles, d) {
continue
}
e.appendRow([]interface{}{
Expand Down Expand Up @@ -283,8 +283,10 @@ func (e *ShowExec) fetchShowOpenTables() error {

func (e *ShowExec) fetchShowTables() error {
checker := privilege.GetPrivilegeManager(e.ctx)
if checker != nil && e.ctx.GetSessionVars().User != nil && !checker.DBIsVisible(e.DBName.O) {
return e.dbAccessDenied()
if checker != nil && e.ctx.GetSessionVars().User != nil {
if !checker.DBIsVisible(e.ctx.GetSessionVars().ActiveRoles, e.DBName.O) {
return e.dbAccessDenied()
}
}
if !e.is.SchemaExists(e.DBName) {
return ErrBadDB.GenWithStackByArgs(e.DBName)
Expand Down Expand Up @@ -319,8 +321,10 @@ func (e *ShowExec) fetchShowTables() error {

func (e *ShowExec) fetchShowTableStatus() error {
checker := privilege.GetPrivilegeManager(e.ctx)
if checker != nil && e.ctx.GetSessionVars().User != nil && !checker.DBIsVisible(e.DBName.O) {
return e.dbAccessDenied()
if checker != nil && e.ctx.GetSessionVars().User != nil {
if !checker.DBIsVisible(e.ctx.GetSessionVars().ActiveRoles, e.DBName.O) {
return e.dbAccessDenied()
}
}
if !e.is.SchemaExists(e.DBName) {
return ErrBadDB.GenWithStackByArgs(e.DBName)
Expand Down Expand Up @@ -883,8 +887,10 @@ func appendPartitionInfo(partitionInfo *model.PartitionInfo, buf *bytes.Buffer)
// fetchShowCreateDatabase composes show create database result.
func (e *ShowExec) fetchShowCreateDatabase() error {
checker := privilege.GetPrivilegeManager(e.ctx)
if checker != nil && e.ctx.GetSessionVars().User != nil && !checker.DBIsVisible(fmt.Sprint(e.DBName)) {
return e.dbAccessDenied()
if checker != nil && e.ctx.GetSessionVars().User != nil {
if !checker.DBIsVisible(e.ctx.GetSessionVars().ActiveRoles, e.DBName.String()) {
return e.dbAccessDenied()
}
}
db, ok := e.is.SchemaByName(e.DBName)
if !ok {
Expand Down
6 changes: 4 additions & 2 deletions executor/simple.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -363,8 +363,10 @@ func (e *SimpleExec) executeUse(s *ast.UseStmt) error {
dbname := model.NewCIStr(s.DBName)

checker := privilege.GetPrivilegeManager(e.ctx)
if checker != nil && e.ctx.GetSessionVars().User != nil && !checker.DBIsVisible(fmt.Sprint(dbname)) {
return e.dbAccessDenied(dbname.O)
if checker != nil && e.ctx.GetSessionVars().User != nil {
if !checker.DBIsVisible(e.ctx.GetSessionVars().ActiveRoles, dbname.String()) {
return e.dbAccessDenied(dbname.O)
}
}

dbinfo, exists := e.is.SchemaByName(dbname)
Expand Down
2 changes: 1 addition & 1 deletion privilege/privilege.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ type Manager interface {
ConnectionVerification(user, host string, auth, salt []byte) (string, string, bool)

// DBIsVisible returns true is the database is visible to current user.
DBIsVisible(db string) bool
DBIsVisible(activeRole []*auth.RoleIdentity, db string) bool

// UserPrivilegesTable provide data for INFORMATION_SCHEMA.USERS_PRIVILEGE table.
UserPrivilegesTable() [][]types.Datum
Expand Down
13 changes: 11 additions & 2 deletions privilege/privileges/privileges.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,12 +160,21 @@ func (p *UserPrivileges) ConnectionVerification(user, host string, authenticatio
}

// DBIsVisible implements the Manager interface.
func (p *UserPrivileges) DBIsVisible(db string) bool {
func (p *UserPrivileges) DBIsVisible(activeRoles []*auth.RoleIdentity, db string) bool {
if SkipWithGrant {
return true
}
mysqlPriv := p.Handle.Get()
return mysqlPriv.DBIsVisible(p.user, p.host, db)
if mysqlPriv.DBIsVisible(p.user, p.host, db) {
Copy link

Choose a reason for hiding this comment

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

If r_1 has privilege, r_2 doesn't have, and has relationship like r_1 -> r_2 -> user, user should have privilege to visit db.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you mean user has role r_2
r_2 has role r_1 ?

I think activeRoles contains both r_1 and r_2 ? @imtbkcat

Copy link

Choose a reason for hiding this comment

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

ctx.ActiveRoles just contain r_1, you could use MySQLPrivileges.FindAllRole to get r_1 and r_2.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.
PTAL @imtbkcat

return true
}
allRoles := mysqlPriv.FindAllRole(activeRoles)
for _, role := range allRoles {
if mysqlPriv.DBIsVisible(role.Username, role.Hostname, db) {
return true
}
}
return false
}

// UserPrivilegesTable implements the Manager interface.
Expand Down
17 changes: 16 additions & 1 deletion privilege/privileges/privileges_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,7 @@ func (s *testPrivilegeSuite) TestCheckAuthenticate(c *C) {
mustExec(c, se1, "drop user '[email protected]'@'localhost'")
}

func (s *testPrivilegeSuite) TestUseDb(c *C) {
func (s *testPrivilegeSuite) TestUseDB(c *C) {

se := newSession(c, s.store, s.dbName)
// high privileged user
Expand All @@ -465,6 +465,21 @@ func (s *testPrivilegeSuite) TestUseDb(c *C) {
c.Assert(se.Auth(&auth.UserIdentity{Username: "usenobody", Hostname: "localhost", AuthUsername: "usenobody", AuthHostname: "%"}, nil, nil), IsTrue)
_, err = se.Execute(context.Background(), "use mysql")
c.Assert(err, IsNil)

// test `use db` for role.
c.Assert(se.Auth(&auth.UserIdentity{Username: "usesuper", Hostname: "localhost", AuthUsername: "usesuper", AuthHostname: "%"}, nil, nil), IsTrue)
mustExec(c, se, `CREATE DATABASE app_db`)
mustExec(c, se, `CREATE ROLE 'app_developer'`)
mustExec(c, se, `GRANT ALL ON app_db.* TO 'app_developer'`)
mustExec(c, se, `CREATE USER 'dev'@'localhost'`)
mustExec(c, se, `GRANT 'app_developer' TO 'dev'@'localhost'`)
mustExec(c, se, `SET DEFAULT ROLE 'app_developer' TO 'dev'@'localhost'`)
mustExec(c, se, `FLUSH PRIVILEGES`)
c.Assert(se.Auth(&auth.UserIdentity{Username: "dev", Hostname: "localhost", AuthUsername: "dev", AuthHostname: "localhost"}, nil, nil), IsTrue)
_, err = se.Execute(context.Background(), "use app_db")
c.Assert(err, IsNil)
_, err = se.Execute(context.Background(), "use mysql")
c.Assert(err, NotNil)
}

func (s *testPrivilegeSuite) TestSetGlobal(c *C) {
Expand Down