Merge pull request cockroachdb#58512 from rafiss/backport20.2-58254

release-20.2: sql: Fix privilege checks to consider direct/indirect roles

pkg/sql/exec_util.go

@@ -1072,6 +1072,14 @@ func golangFillQueryArguments(args ...interface{}) (tree.Datums, error) {
 				switch {
 				case val.IsNil():
 					d = tree.DNull
+				case val.Type().Elem().Kind() == reflect.String:
+					a := tree.NewDArray(types.String)
+					for v := 0; v < val.Len(); v++ {
+						if err := a.Append(tree.NewDString(val.Index(v).String())); err != nil {
+							return nil, err
+						}
+					}
+					d = a
 				case val.Type().Elem().Kind() == reflect.Uint8:
 					d = tree.NewDBytes(tree.DBytes(val.Bytes()))
 				}

pkg/sql/faketreeeval/evalctx.go

@@ -114,6 +114,13 @@ func (ep *DummyEvalPlanner) UnsafeDeleteNamespaceEntry(
 	return errors.WithStack(errEvalPlanner)
 }
 
+// MemberOfWithAdminOption is part of the EvalPlanner interface.
+func (ep *DummyEvalPlanner) MemberOfWithAdminOption(
+	ctx context.Context, member string,
+) (map[string]bool, error) {
+	return nil, errors.WithStack(errEvalPlanner)
+}
+
 var _ tree.EvalPlanner = &DummyEvalPlanner{}
 
 var errEvalPlanner = pgerror.New(pgcode.ScalarOperationCannotRunWithoutFullSessionContext,

pkg/sql/logictest/testdata/logic_test/privilege_builtins

@@ -1054,3 +1054,77 @@ query B
 SELECT has_column_privilege('hcp_test'::REGCLASS, 4, 'SELECT')
 ----
 true
+
+# Regression test for #58254. Tests privilege inheritance for roles (direct and indirect).
+
+user root
+
+statement ok
+DROP DATABASE IF EXISTS my_db;
+CREATE DATABASE my_db;
+
+statement ok
+CREATE ROLE my_role;
+
+statement ok
+GRANT CREATE ON DATABASE my_db TO my_role;
+GRANT my_role TO testuser;
+
+user testuser
+
+statement ok
+use my_db
+
+statement ok
+CREATE SCHEMA s;
+CREATE TABLE s.t()
+
+# Privilege check on direct member of role with CREATE privilege granted.
+query B
+SELECT has_schema_privilege('testuser', 's', 'create')
+----
+true
+
+# Privilege check on direct member of role without USAGE privilege granted.
+query B
+SELECT has_schema_privilege('testuser', 's', 'usage')
+----
+false
+
+# Confirm privilege checks on all objects.
+query BBB
+SELECT has_database_privilege('testuser', 'my_db', 'create'),
+       has_schema_privilege('testuser', 'public', 'usage'),
+       has_table_privilege('testuser', 's.t', 'select')
+----
+true false false
+
+user root
+
+statement ok
+use my_db;
+CREATE USER testuser2;
+GRANT testuser TO testuser2;
+GRANT ALL ON SCHEMA s TO testuser2
+
+user testuser2
+
+statement ok
+use my_db
+
+# Make sure testuser only has CREATE privilege on schema s but testuser2 has both CREATE and USAGE.
+query BBBB
+SELECT has_schema_privilege('testuser', 's', 'create'),
+       has_schema_privilege('testuser', 's', 'usage'),
+       has_schema_privilege('testuser2', 's', 'create'),
+       has_schema_privilege('testuser2', 's', 'usage')
+----
+true false true true
+
+# Confirm privilege checks on all objects with testuser2, should match testuser.
+query BBB
+SELECT has_database_privilege('testuser2', 'my_db', 'create'),
+       has_schema_privilege('testuser2', 'public', 'usage'),
+       has_table_privilege('testuser2', 's.t', 'select')
+----
+true false false

pkg/sql/sem/builtins/pg_builtins.go

@@ -521,13 +521,25 @@ func evalPrivilegeCheck(
 	if withGrantOpt {
 		privChecks = append(privChecks, privilege.GRANT)
 	}
+
+	allRoleMemberships, err := ctx.Planner.MemberOfWithAdminOption(ctx.Context, user)
+	if err != nil {
+		return nil, err
+	}
+
+	// Slice containing all roles user is a direct and indirect member of.
+	allRoles := []string{security.PublicRole, user}
+	for role := range allRoleMemberships {
+		allRoles = append(allRoles, role)
+	}
+
 	for _, p := range privChecks {
 		query := fmt.Sprintf(`
 			SELECT bool_or(privilege_type IN ('%s', '%s')) IS TRUE
-			FROM information_schema.%s WHERE grantee IN ($1, $2) AND %s`,
+			FROM information_schema.%s WHERE grantee = ANY ($1) AND %s`,
 			privilege.ALL, p, infoTable, pred)
 		r, err := ctx.InternalExecutor.QueryRow(
-			ctx.Ctx(), "eval-privilege-check", ctx.Txn, query, security.PublicRole, user,
+			ctx.Ctx(), "eval-privilege-check", ctx.Txn, query, allRoles,
 		)
 		if err != nil {
 			return nil, err

pkg/sql/sem/tree/eval.go

@@ -3017,6 +3017,14 @@ type EvalPlanner interface {
 		descID int64,
 		force bool,
 	) error
+
+	// MemberOfWithAdminOption is used to collect a list of roles (direct and
+	// indirect) that the member is part of. See the comment on the planner
+	// implementation in authorization.go
+	MemberOfWithAdminOption(
+		ctx context.Context,
+		member string,
+	) (map[string]bool, error)
 }
 
 // EvalSessionAccessor is a limited interface to access session variables.