server: fix UserSQLRoles to account for global privileges

Release note: None
cockroachdb · Jan 18, 2023 · 849f846 · 849f846
1 parent 582d845
commit 849f846
Show file tree

Hide file tree

Showing 2 changed files with 19 additions and 7 deletions.
diff --git a/pkg/server/user.go b/pkg/server/user.go
@@ -14,6 +14,7 @@ import (
 	"context"
 
 	"github.com/cockroachdb/cockroach/pkg/server/serverpb"
+	"github.com/cockroachdb/cockroach/pkg/sql/privilege"
 	"github.com/cockroachdb/cockroach/pkg/sql/roleoption"
 )
 
@@ -31,13 +32,23 @@ func (s *baseStatusServer) UserSQLRoles(
 
 	var resp serverpb.UserSQLRolesResponse
 	if !isAdmin {
-		for name := range roleoption.ByName {
-			hasRole, err := s.privilegeChecker.hasRoleOption(ctx, username, roleoption.ByName[name])
+		for _, privKind := range privilege.GlobalPrivileges {
+			privName := privKind.String()
+			hasPriv := s.privilegeChecker.checkHasGlobalPrivilege(ctx, username, privKind)
+			if hasPriv {
+				resp.Roles = append(resp.Roles, privName)
+				continue
+			}
+			roleOpt, ok := roleoption.ByName[privName]
+			if !ok {
+				continue
+			}
+			hasRole, err := s.privilegeChecker.hasRoleOption(ctx, username, roleOpt)
 			if err != nil {
 				return nil, err
 			}
 			if hasRole {
-				resp.Roles = append(resp.Roles, name)
+				resp.Roles = append(resp.Roles, privName)
 			}
 		}
 	} else {

diff --git a/pkg/server/user_test.go b/pkg/server/user_test.go
@@ -95,24 +95,25 @@ func TestSQLRolesAPI(t *testing.T) {
 	require.NoError(t, err)
 	require.Equal(t, expRoles, res.Roles)
 
-	// One role added to the non-admin user.
+	// Role option and global privilege added to the non-admin user.
 	db.Exec(t, fmt.Sprintf("ALTER USER %s VIEWACTIVITY", authenticatedUserNameNoAdmin().Normalized()))
-	expRoles = []string{"VIEWACTIVITY"}
+	db.Exec(t, fmt.Sprintf("GRANT SYSTEM MODIFYCLUSTERSETTING TO %s", authenticatedUserNameNoAdmin().Normalized()))
+	expRoles = []string{"MODIFYCLUSTERSETTING", "VIEWACTIVITY"}
 	err = getStatusJSONProtoWithAdminOption(s, "sqlroles", &res, false)
 	require.NoError(t, err)
 	require.Equal(t, expRoles, res.Roles)
 
 	// Two roles added to the non-admin user.
 	db.Exec(t, fmt.Sprintf("ALTER USER %s VIEWACTIVITYREDACTED", authenticatedUserNameNoAdmin().Normalized()))
-	expRoles = []string{"VIEWACTIVITY", "VIEWACTIVITYREDACTED"}
+	expRoles = []string{"MODIFYCLUSTERSETTING", "VIEWACTIVITY", "VIEWACTIVITYREDACTED"}
 	err = getStatusJSONProtoWithAdminOption(s, "sqlroles", &res, false)
 	sort.Strings(res.Roles)
 	require.NoError(t, err)
 	require.Equal(t, expRoles, res.Roles)
 
 	// Remove one role from non-admin user.
 	db.Exec(t, fmt.Sprintf("ALTER USER %s NOVIEWACTIVITY", authenticatedUserNameNoAdmin().Normalized()))
-	expRoles = []string{"VIEWACTIVITYREDACTED"}
+	expRoles = []string{"MODIFYCLUSTERSETTING", "VIEWACTIVITYREDACTED"}
 	err = getStatusJSONProtoWithAdminOption(s, "sqlroles", &res, false)
 	require.NoError(t, err)
 	require.Equal(t, expRoles, res.Roles)