privilege: fix show grants output for active roles dynamic privs (p…

…ingcap#27218)
dgfug · Aug 24, 2021 · 684057f · 684057f
1 parent 74fd4f1
commit 684057f
Show file tree

Hide file tree

Showing 5 changed files with 141 additions and 30 deletions.
diff --git a/executor/builder.go b/executor/builder.go
@@ -29,7  29,6 @@ import (
 	"github.com/pingcap/failpoint"
 	"github.com/pingcap/kvproto/pkg/diagnosticspb"
 	"github.com/pingcap/parser/ast"
-	"github.com/pingcap/parser/auth"
 	"github.com/pingcap/parser/model"
 	"github.com/pingcap/parser/mysql"
 	"github.com/pingcap/tidb/distsql"
@@ -740,14  739,6 @@ func (b *executorBuilder) buildShow(v *plannercore.PhysicalShow) Executor {
 		GlobalScope:  v.GlobalScope,
 		Extended:     v.Extended,
 	}
-	if e.Tp == ast.ShowGrants && e.User == nil {
-		// The input is a "show grants" statement, fulfill the user and roles field.
-		// Note: "show grants" result are different from "show grants for current_user",
-		// The former determine privileges with roles, while the later doesn't.
-		vars := e.ctx.GetSessionVars()
-		e.User = &auth.UserIdentity{Username: vars.User.AuthUsername, Hostname: vars.User.AuthHostname}
-		e.Roles = vars.ActiveRoles
-	}
 	if e.Tp == ast.ShowMasterStatus {
 		// show master status need start ts.
 		if _, err := e.ctx.Txn(true); err != nil {

diff --git a/executor/show.go b/executor/show.go
@@ -1414,30  1414,34 @@ func (e *ShowExec) fetchShowCreateUser(ctx context.Context) error {
 }
 
 func (e *ShowExec) fetchShowGrants() error {
-	// Get checker
 	vars := e.ctx.GetSessionVars()
 	checker := privilege.GetPrivilegeManager(e.ctx)
 	if checker == nil {
 		return errors.New("miss privilege checker")
 	}
-	sessVars := e.ctx.GetSessionVars()
-	if !e.User.CurrentUser {
-		userName := sessVars.User.AuthUsername
-		hostName := sessVars.User.AuthHostname
 	if e.User == nil || e.User.CurrentUser {
 		// The input is a "SHOW GRANTS" statement with no users *or* SHOW GRANTS FOR CURRENT_USER()
 		// In these cases we include the active roles for showing privileges.
 		e.User = &auth.UserIdentity{Username: vars.User.AuthUsername, Hostname: vars.User.AuthHostname}
 		e.Roles = vars.ActiveRoles
 	} else {
 		userName := vars.User.AuthUsername
 		hostName := vars.User.AuthHostname
 		// Show grant user requires the SELECT privilege on mysql schema.
 		// Ref https://dev.mysql.com/doc/refman/8.0/en/show-grants.html
 		if userName != e.User.Username || hostName != e.User.Hostname {
-			activeRoles := sessVars.ActiveRoles
-			if !checker.RequestVerification(activeRoles, mysql.SystemDB, "", "", mysql.SelectPriv) {
 			if !checker.RequestVerification(vars.ActiveRoles, mysql.SystemDB, "", "", mysql.SelectPriv) {
 				return ErrDBaccessDenied.GenWithStackByArgs(userName, hostName, mysql.SystemDB)
 			}
 		}
-	}
-	for _, r := range e.Roles {
-		if r.Hostname == "" {
-			r.Hostname = "%"
-		}
-		if !checker.FindEdge(e.ctx, r, e.User) {
-			return ErrRoleNotGranted.GenWithStackByArgs(r.String(), e.User.String())
 		// This is for the syntax SHOW GRANTS FOR x USING role
 		for _, r := range e.Roles {
 			if r.Hostname == "" {
 				r.Hostname = "%"
 			}
 			if !checker.FindEdge(e.ctx, r, e.User) {
 				return ErrRoleNotGranted.GenWithStackByArgs(r.String(), e.User.String())
 			}
 		}
 	}
 	gs, err := checker.ShowGrants(e.ctx, e.User, e.Roles)

diff --git a/executor/show_test.go b/executor/show_test.go
@@ -267,7  267,8 @@ func (s *testSuite5) TestIssue10549(c *C) {
 	c.Assert(tk.Se.Auth(&auth.UserIdentity{Username: "dev", Hostname: "%", AuthUsername: "dev", AuthHostname: "%"}, nil, nil), IsTrue)
 	tk.MustQuery("SHOW DATABASES;").Check(testkit.Rows("INFORMATION_SCHEMA", "newdb"))
 	tk.MustQuery("SHOW GRANTS;").Check(testkit.Rows("GRANT USAGE ON *.* TO 'dev'@'%'", "GRANT ALL PRIVILEGES ON newdb.* TO 'dev'@'%'", "GRANT 'app_developer'@'%' TO 'dev'@'%'"))
-	tk.MustQuery("SHOW GRANTS FOR CURRENT_USER").Check(testkit.Rows("GRANT USAGE ON *.* TO 'dev'@'%'", "GRANT 'app_developer'@'%' TO 'dev'@'%'"))
 	tk.MustQuery("SHOW GRANTS FOR CURRENT_USER").Check(testkit.Rows("GRANT USAGE ON *.* TO 'dev'@'%'", "GRANT ALL PRIVILEGES ON newdb.* TO 'dev'@'%'", "GRANT 'app_developer'@'%' TO 'dev'@'%'"))
 	tk.MustQuery("SHOW GRANTS FOR dev").Check(testkit.Rows("GRANT USAGE ON *.* TO 'dev'@'%'", "GRANT 'app_developer'@'%' TO 'dev'@'%'"))
 }
 
 func (s *testSuite5) TestIssue11165(c *C) {

diff --git a/privilege/privileges/cache.go b/privilege/privileges/cache.go
@@ -1283,23  1283,47 @@ func (p *MySQLPrivilege) showGrants(user, host string, roles []*auth.RoleIdentit
 		gs = append(gs, s)
 	}
 
-	// Show dynamic privileges
-	var dynamicPrivs, grantableDynamicPrivs []string
 	// If the SHOW GRANTS is for the current user, there might be activeRoles (allRoles)
 	// The convention is to merge the Dynamic privileges assigned to the user with
 	// inherited dynamic privileges from those roles
 	dynamicPrivsMap := make(map[string]bool) // privName, grantable
 	for _, record := range p.Dynamic[user] {
 		if record.fullyMatch(user, host) {
-			if record.GrantOption {
-				grantableDynamicPrivs = append(grantableDynamicPrivs, record.PrivilegeName)
-			} else {
-				dynamicPrivs = append(dynamicPrivs, record.PrivilegeName)
 			dynamicPrivsMap[record.PrivilegeName] = record.GrantOption
 		}
 	}
 	for _, r := range allRoles {
 		for _, record := range p.Dynamic[r.Username] {
 			if record.fullyMatch(r.Username, r.Hostname) {
 				// If the record already exists in the map and it's grantable
 				// skip doing anything, because we might inherit a non-grantable permission
 				// from a role, and don't want to clobber the existing privilege.
 				if grantable, ok := dynamicPrivsMap[record.PrivilegeName]; ok && grantable {
 					continue
 				}
 				dynamicPrivsMap[record.PrivilegeName] = record.GrantOption
 			}
 		}
 	}
 
 	// Convert the map to a slice so it can be sorted to be deterministic and joined
 	var dynamicPrivs, grantableDynamicPrivs []string
 	for privName, grantable := range dynamicPrivsMap {
 		if grantable {
 			grantableDynamicPrivs = append(grantableDynamicPrivs, privName)
 		} else {
 			dynamicPrivs = append(dynamicPrivs, privName)
 		}
 	}
 
 	// Merge the DYNAMIC privs into a line for non-grantable and then grantable.
 	if len(dynamicPrivs) > 0 {
 		sort.Strings(dynamicPrivs)
 		s := fmt.Sprintf("GRANT %s ON *.* TO '%s'@'%s'", strings.Join(dynamicPrivs, ","), user, host)
 		gs = append(gs, s)
 	}
 	if len(grantableDynamicPrivs) > 0 {
 		sort.Strings(grantableDynamicPrivs)
 		s := fmt.Sprintf("GRANT %s ON *.* TO '%s'@'%s' WITH GRANT OPTION", strings.Join(grantableDynamicPrivs, ","), user, host)
 		gs = append(gs, s)
 	}

diff --git a/privilege/privileges/privileges_test.go b/privilege/privileges/privileges_test.go
@@ -2085,3  2085,94 @@ func TestGrantReferences(t *testing.T) {
 	tk.MustExec("DROP USER referencesUser")
 	tk.MustExec("DROP SCHEMA reftestdb")
 }
 
 // https://github.com/pingcap/tidb/issues/27213
 func TestShowGrantsWithRolesAndDynamicPrivs(t *testing.T) {
 	t.Parallel()
 
 	store, clean := newStore(t)
 	defer clean()
 
 	tk := testkit.NewTestKit(t, store)
 	tk.MustExec("CREATE ROLE tsg_r1")
 	tk.MustExec("CREATE USER tsg_u1, tsg_u2")
 	tk.MustExec("GRANT CONNECTION_ADMIN, ROLE_ADMIN, SYSTEM_VARIABLES_ADMIN, PROCESS ON *.* TO tsg_r1")
 	tk.MustExec("GRANT CONNECTION_ADMIN ON *.* TO tsg_u1 WITH GRANT OPTION") // grant a superior privilege to the user
 	tk.MustExec("GRANT CONNECTION_ADMIN ON *.* TO tsg_u2 WITH GRANT OPTION") // grant a superior privilege to the user
 	tk.MustExec("GRANT ROLE_ADMIN ON *.* TO tsg_u1")
 	tk.MustExec("GRANT ROLE_ADMIN ON *.* TO tsg_u2")
 	tk.MustExec("GRANT ROLE_ADMIN ON *.* TO tsg_r1 WITH GRANT OPTION") // grant a superior privilege to the role
 	tk.MustExec("GRANT CONFIG ON *.* TO tsg_r1")                       // grant a static privilege to the role
 
 	tk.MustExec("GRANT tsg_r1 TO tsg_u1, tsg_u2")    // grant the role to both users
 	tk.MustExec("SET DEFAULT ROLE tsg_r1 TO tsg_u1") // u1 has the role by default, but results should be identical.
 
 	// login as tsg_u1
 	tk.Session().Auth(&auth.UserIdentity{
 		Username: "tsg_u1",
 		Hostname: "localhost",
 	}, nil, nil)
 	tk.MustQuery("SHOW GRANTS").Check(testkit.Rows(
 		"GRANT PROCESS,CONFIG ON *.* TO 'tsg_u1'@'%'",
 		"GRANT 'tsg_r1'@'%' TO 'tsg_u1'@'%'",
 		"GRANT SYSTEM_VARIABLES_ADMIN ON *.* TO 'tsg_u1'@'%'",
 		"GRANT CONNECTION_ADMIN,ROLE_ADMIN ON *.* TO 'tsg_u1'@'%' WITH GRANT OPTION",
 	))
 	tk.MustQuery("SHOW GRANTS FOR CURRENT_USER()").Check(testkit.Rows(
 		"GRANT PROCESS,CONFIG ON *.* TO 'tsg_u1'@'%'",
 		"GRANT 'tsg_r1'@'%' TO 'tsg_u1'@'%'",
 		"GRANT SYSTEM_VARIABLES_ADMIN ON *.* TO 'tsg_u1'@'%'",
 		"GRANT CONNECTION_ADMIN,ROLE_ADMIN ON *.* TO 'tsg_u1'@'%' WITH GRANT OPTION",
 	))
 	tk.MustQuery("SHOW GRANTS FOR 'tsg_u1'").Check(testkit.Rows(
 		"GRANT USAGE ON *.* TO 'tsg_u1'@'%'",
 		"GRANT 'tsg_r1'@'%' TO 'tsg_u1'@'%'",
 		"GRANT ROLE_ADMIN ON *.* TO 'tsg_u1'@'%'",
 		"GRANT CONNECTION_ADMIN ON *.* TO 'tsg_u1'@'%' WITH GRANT OPTION",
 	))
 
 	// login as tsg_u2   SET ROLE
 	tk.Session().Auth(&auth.UserIdentity{
 		Username: "tsg_u2",
 		Hostname: "localhost",
 	}, nil, nil)
 	tk.MustQuery("SHOW GRANTS").Check(testkit.Rows(
 		"GRANT USAGE ON *.* TO 'tsg_u2'@'%'",
 		"GRANT 'tsg_r1'@'%' TO 'tsg_u2'@'%'",
 		"GRANT ROLE_ADMIN ON *.* TO 'tsg_u2'@'%'",
 		"GRANT CONNECTION_ADMIN ON *.* TO 'tsg_u2'@'%' WITH GRANT OPTION",
 	))
 	tk.MustQuery("SHOW GRANTS FOR CURRENT_USER()").Check(testkit.Rows(
 		"GRANT USAGE ON *.* TO 'tsg_u2'@'%'",
 		"GRANT 'tsg_r1'@'%' TO 'tsg_u2'@'%'",
 		"GRANT ROLE_ADMIN ON *.* TO 'tsg_u2'@'%'",
 		"GRANT CONNECTION_ADMIN ON *.* TO 'tsg_u2'@'%' WITH GRANT OPTION",
 	))
 	// This should not show the privileges gained from (default) roles
 	tk.MustQuery("SHOW GRANTS FOR 'tsg_u2'").Check(testkit.Rows(
 		"GRANT USAGE ON *.* TO 'tsg_u2'@'%'",
 		"GRANT 'tsg_r1'@'%' TO 'tsg_u2'@'%'",
 		"GRANT ROLE_ADMIN ON *.* TO 'tsg_u2'@'%'",
 		"GRANT CONNECTION_ADMIN ON *.* TO 'tsg_u2'@'%' WITH GRANT OPTION",
 	))
 	tk.MustExec("SET ROLE tsg_r1")
 	tk.MustQuery("SHOW GRANTS").Check(testkit.Rows(
 		"GRANT PROCESS,CONFIG ON *.* TO 'tsg_u2'@'%'",
 		"GRANT 'tsg_r1'@'%' TO 'tsg_u2'@'%'",
 		"GRANT SYSTEM_VARIABLES_ADMIN ON *.* TO 'tsg_u2'@'%'",
 		"GRANT CONNECTION_ADMIN,ROLE_ADMIN ON *.* TO 'tsg_u2'@'%' WITH GRANT OPTION",
 	))
 	tk.MustQuery("SHOW GRANTS FOR CURRENT_USER()").Check(testkit.Rows(
 		"GRANT PROCESS,CONFIG ON *.* TO 'tsg_u2'@'%'",
 		"GRANT 'tsg_r1'@'%' TO 'tsg_u2'@'%'",
 		"GRANT SYSTEM_VARIABLES_ADMIN ON *.* TO 'tsg_u2'@'%'",
 		"GRANT CONNECTION_ADMIN,ROLE_ADMIN ON *.* TO 'tsg_u2'@'%' WITH GRANT OPTION",
 	))
 	// This should not show the privileges gained from SET ROLE tsg_r1.
 	tk.MustQuery("SHOW GRANTS FOR 'tsg_u2'").Check(testkit.Rows(
 		"GRANT USAGE ON *.* TO 'tsg_u2'@'%'",
 		"GRANT 'tsg_r1'@'%' TO 'tsg_u2'@'%'",
 		"GRANT ROLE_ADMIN ON *.* TO 'tsg_u2'@'%'",
 		"GRANT CONNECTION_ADMIN ON *.* TO 'tsg_u2'@'%' WITH GRANT OPTION",
 	))
 }