Merge #104119

104119: sql: SHOW GRANTS includes inherited roles r=postamar a=postamar

  Previously, SHOW GRANTS would only list explicitly-granted privileges.
  This commit changes its output to include privileges inherited by role
  membership.
  
  Consider for example:
  
      CREATE DATABASE d1;
      CREATE ROLE r1;
      CREATE USER u1;
      GRANT r1 TO u1;
      SET database = d1;
      CREATE TABLE t1 (id INT8);
      GRANT ALL ON TABLE t1 TO r1;
  
  We want `SHOW GRANTS ON TABLE t1 FOR u1` to list the ALL privilege that
  u1 inherits from r1.
  
  For this purpose this commit also adds a new `kv_inherited_role_members`
  virtual table in `crdb_internal` which is essentially a recursive
  self-join of `system.role_members` run by the node user.
  
  This commit also changes the output of `SHOW GRANTS ON ROLE` to list
  both explicit and implicit grantees: the former are the direct result of
  a `GRANT ... TO ...` statement, the latter comprise also all the roles
  which inherit the privileges following further role membership
  relationships. For this purpose it also uses the new virtual table and
  therefore the current user no longer requires SELECT privileges on the
  `system.role_members` table.
  
  Fixes #97299.
  
  Release note (sql change): SHOW GRANTS now lists not just privileges
  explicitly granted to each role, but also roles which inherit from
  those. SHOW GRANTS ON ROLE statements no longer require any privileges
  and also lists implicit grantees.


Co-authored-by: Marius Posta <[email protected]>

pkg/bench/rttanalysis/grant_revoke_role_bench_test.go

@@ -33,6 +33,75 @@ CREATE ROLE c;`,
 	})
 }
 
+func BenchmarkShowGrants(b *testing.B) { reg.Run(b) }
+func init() {
+	reg.Register("ShowGrants", []RoundTripBenchTestCase{
+		{
+			Name: "grant 2 roles",
+			Setup: `
+CREATE DATABASE db;
+CREATE ROLE a;
+CREATE ROLE b;
+CREATE ROLE c;
+GRANT a TO b;
+GRANT b TO c;
+GRANT ALL ON DATABASE db TO c;
+GRANT DROP ON DATABASE db TO b;
+GRANT CONNECT ON DATABASE db TO a;
+`,
+			Stmt: "SHOW GRANTS ON DATABASE db FOR c",
+			Reset: `
+DROP DATABASE db;
+DROP ROLE a,b,c;
+`,
+		},
+		{
+			Name: "grant 3 roles",
+			Setup: `
+CREATE DATABASE db;
+CREATE ROLE a;
+CREATE ROLE b;
+CREATE ROLE c;
+CREATE ROLE d;
+GRANT a TO b;
+GRANT b TO c;
+GRANT c TO d;
+GRANT ALL ON DATABASE db TO c;
+GRANT DROP ON DATABASE db TO b;
+GRANT CONNECT ON DATABASE db TO a;
+`,
+			Stmt: "SHOW GRANTS ON DATABASE db FOR d",
+			Reset: `
+DROP DATABASE db;
+DROP ROLE a,b,c,d;
+`,
+		},
+		{
+			Name: "grant 4 roles",
+			Setup: `
+CREATE DATABASE db;
+CREATE ROLE a;
+CREATE ROLE b;
+CREATE ROLE c;
+CREATE ROLE d;
+CREATE ROLE e;
+GRANT a TO b;
+GRANT b TO c;
+GRANT c TO d;
+GRANT d TO e;
+GRANT ALL ON DATABASE db TO c;
+GRANT DROP ON DATABASE db TO b;
+GRANT CONNECT ON DATABASE db TO a;
+`,
+			Stmt: "SHOW GRANTS ON DATABASE db FOR d",
+			Reset: `
+DROP DATABASE db;
+DROP ROLE a,b,c,d,e;
+`,
+		},
+	})
+}
+
 func BenchmarkRevokeRole(b *testing.B) { reg.Run(b) }
 func init() {
 	reg.Register("RevokeRole", []RoundTripBenchTestCase{

pkg/bench/rttanalysis/testdata/benchmark_expectations

@@ -91,6 +91,9 @@ exp,benchmark
 21,Revoke/revoke_all_on_3_tables
 14,RevokeRole/revoke_1_role
 16,RevokeRole/revoke_2_roles
+10,ShowGrants/grant_2_roles
+11,ShowGrants/grant_3_roles
+12,ShowGrants/grant_4_roles
 1,SystemDatabaseQueries/select_system.users_with_empty_database_Name
 1,SystemDatabaseQueries/select_system.users_with_schema_Name
 1,SystemDatabaseQueries/select_system.users_without_schema_Name

pkg/ccl/logictestccl/testdata/logic_test/crdb_internal_tenant

@@ -64,6 +64,7 @@ crdb_internal  kv_catalog_descriptor                   table  admin  NULL  NULL
 crdb_internal  kv_catalog_namespace                    table  admin  NULL  NULL
 crdb_internal  kv_catalog_zones                        table  admin  NULL  NULL
 crdb_internal  kv_dropped_relations                    view   admin  NULL  NULL
+crdb_internal  kv_inherited_role_members               table  admin  NULL  NULL
 crdb_internal  kv_node_liveness                        table  admin  NULL  NULL
 crdb_internal  kv_node_status                          table  admin  NULL  NULL
 crdb_internal  kv_store_status                         table  admin  NULL  NULL

pkg/cli/zip_test.go

@@ -85,6 +85,7 @@ table_name NOT IN (
 	'kv_catalog_namespace',
 	'kv_catalog_zones',
 	'kv_dropped_relations',
+	'kv_inherited_role_members',
 	'lost_descriptors_with_data',
 	'table_columns',
 	'table_row_statistics',

pkg/sql/crdb_internal.go

@@ -207,6 +207,7 @@ var crdbInternal = virtualSchema{
 		catconstants.CrdbInternalTenantUsageDetailsViewID:           crdbInternalTenantUsageDetailsView,
 		catconstants.CrdbInternalPgCatalogTableIsImplementedTableID: crdbInternalPgCatalogTableIsImplementedTable,
 		catconstants.CrdbInternalShowTenantCapabilitiesCacheTableID: crdbInternalShowTenantCapabilitiesCache,
+		catconstants.CrdbInternalInheritedRoleMembersTableID:        crdbInternalInheritedRoleMembers,
 	},
 	validWithNoDatabaseContext: true,
 }
@@ -8034,3 +8035,71 @@ CREATE TABLE crdb_internal.node_tenant_capabilities_cache (
 		return nil
 	},
 }
+
+var crdbInternalInheritedRoleMembers = virtualSchemaTable{
+	comment: `table listing transitive closure of system.role_members`,
+	schema: `
+CREATE TABLE crdb_internal.kv_inherited_role_members (
+  role               STRING,
+  inheriting_member  STRING,
+  member_is_explicit BOOL,
+  member_is_admin    BOOL
+);`,
+	populate: func(ctx context.Context, p *planner, _ catalog.DatabaseDescriptor, addRow func(...tree.Datum) error) (retErr error) {
+		// Populate explicitMemberships with the contents of system.role_members.
+		// This data structure maps each role to the set of roles it's explicitly
+		// a member of. Consider for example:
+		//     CREATE ROLE a;
+		//     CREATE ROLE b;
+		//     CREATE ROLE c;
+		//     GRANT a TO b;
+		//     GRANT b TO c;
+		// The role `a` is explicitly a member of `b` and the role `c` is
+		// explicitly a member of `b`. The role `c` is also a member of `a`,
+		// but not explicitly, because it inherits the membership through `b`.
+		explicitMemberships := make(map[username.SQLUsername]map[username.SQLUsername]bool)
+		if err := forEachRoleMembership(ctx, p.InternalSQLTxn(), func(role, member username.SQLUsername, isAdmin bool) error {
+			if _, found := explicitMemberships[member]; !found {
+				explicitMemberships[member] = make(map[username.SQLUsername]bool)
+			}
+			explicitMemberships[member][role] = true
+			return nil
+		}); err != nil {
+			return err
+		}
+		// Iterate through all members in order for stability.
+		members := make([]username.SQLUsername, 0, len(explicitMemberships))
+		for m := range explicitMemberships {
+			members = append(members, m)
+		}
+		sort.Slice(members, func(i, j int) bool {
+			return members[i].Normalized() < members[j].Normalized()
+		})
+		for _, member := range members {
+			memberOf, err := p.MemberOfWithAdminOption(ctx, member)
+			if err != nil {
+				return err
+			}
+			explicitRoles := explicitMemberships[member]
+			// Iterate through all roles in order for stability.
+			roles := make([]username.SQLUsername, 0, len(memberOf))
+			for r := range memberOf {
+				roles = append(roles, r)
+			}
+			sort.Slice(roles, func(i, j int) bool {
+				return roles[i].Normalized() < roles[j].Normalized()
+			})
+			for _, r := range roles {
+				if err := addRow(
+					tree.NewDString(r.Normalized()),              // role
+					tree.NewDString(member.Normalized()),         // inheriting_member
+					tree.MakeDBool(tree.DBool(explicitRoles[r])), // member_is_explicit
+					tree.MakeDBool(tree.DBool(memberOf[r])),      // member_is_admin
+				); err != nil {
+					return err
+				}
+			}
+		}
+		return nil
+	},
+}