Merge pull request #108780 from maryliag/backport23.1-108486

release-23.1: server: allow users with VIEWACTIVITY to get console settings

pkg/server/BUILD.bazel

@@ -347,6 +347,7 @@ go_library(
         "@org_golang_google_grpc//codes",
         "@org_golang_google_grpc//metadata",
         "@org_golang_google_grpc//status",
+        "@org_golang_x_exp//slices",
     ] + select({
         "@io_bazel_rules_go//go/platform:aix": [
             "@org_golang_x_sys//unix",

pkg/server/admin.go

@@ -81,6 +81,7 @@ import (
 	"github.com/cockroachdb/errors"
 	gwruntime "github.com/grpc-ecosystem/grpc-gateway/runtime"
 	gwutil "github.com/grpc-ecosystem/grpc-gateway/utilities"
+	"golang.org/x/exp/slices"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/codes"
 	grpcstatus "google.golang.org/grpc/status"
@@ -2020,17 +2021,15 @@ func (s *adminServer) Settings(
 ) (*serverpb.SettingsResponse, error) {
 	ctx = s.AnnotateCtx(ctx)
 
-	keys := req.Keys
-	if len(keys) == 0 {
-		keys = settings.Keys(settings.ForSystemTenant)
-	}
-
 	_, isAdmin, err := s.getUserAndRole(ctx)
 	if err != nil {
 		return nil, serverError(ctx, err)
 	}
 
 	redactValues := true
+	// Only returns non-sensitive settings that are required
+	// for features on DB Console.
+	consoleSettingsOnly := false
 	if isAdmin {
 		// Root accesses can customize the purpose.
 		// This is used by the UI to see all values (local access)
@@ -2039,9 +2038,37 @@ func (s *adminServer) Settings(
 			redactValues = false
 		}
 	} else {
-		// Non-root access cannot see the values in any case.
+		// Non-root access cannot see the values.
+		// Exception: users with VIEWACTIVITY and VIEWACTIVITYREDACTED can see cluster
+		// settings used by the UI Console.
 		if err := s.adminPrivilegeChecker.requireViewClusterSettingOrModifyClusterSettingPermission(ctx); err != nil {
-			return nil, err
+			if err2 := s.adminPrivilegeChecker.requireViewActivityOrViewActivityRedactedPermission(ctx); err2 != nil {
+				// The check for VIEWACTIVITY or VIEWATIVITYREDACTED is a special case so cluster settings from
+				// the console can be returned, but if the user doesn't have them (i.e. err2 != nil), we don't want
+				// to share this error message, so only return `err`.
+				return nil, err
+			}
+			consoleSettingsOnly = true
+		}
+	}
+
+	var settingsKeys []string
+	if !consoleSettingsOnly {
+		settingsKeys = req.Keys
+		if len(settingsKeys) == 0 {
+			settingsKeys = settings.Keys(settings.ForSystemTenant)
+		}
+	} else {
+
+		if len(req.Keys) == 0 {
+			settingsKeys = settings.ConsoleKeys()
+		} else {
+			settingsKeys = []string{}
+			for _, k := range req.Keys {
+				if slices.Contains(settings.ConsoleKeys(), k) {
+					settingsKeys = append(settingsKeys, k)
+				}
+			}
 		}
 	}
 
@@ -2071,7 +2098,7 @@ func (s *adminServer) Settings(
 	}
 
 	resp := serverpb.SettingsResponse{KeyValues: make(map[string]serverpb.SettingsResponse_Value)}
-	for _, k := range keys {
+	for _, k := range settingsKeys {
 		var v settings.Setting
 		var ok bool
 		if redactValues {

pkg/settings/registry.go

@@ -282,6 +282,29 @@ func Keys(forSystemTenant bool) (res []string) {
 	return res
 }
 
+// ConsoleKeys return an array with all cluster settings keys
+// used by the UI Console.
+// This list should only contain settings that have no sensitive
+// information, because it will be returned on `settings` endpoint for
+// users without VIEWCLUSTERSETTING or MODIFYCLUSTERSETTING permission,
+// but that have VIEWACTIVITY or VIEWACTIVITYREDACTED permissions.
+func ConsoleKeys() (res []string) {
+	return []string{
+		"cross_cluster_replication.enabled",
+		"keyvisualizer.enabled",
+		"keyvisualizer.sample_interval",
+		"sql.index_recommendation.drop_unused_duration",
+		"sql.insights.anomaly_detection.latency_threshold",
+		"sql.insights.high_retry_count.threshold",
+		"sql.insights.latency_threshold",
+		"sql.stats.automatic_collection.enabled",
+		"timeseries.storage.resolution_10s.ttl",
+		"timeseries.storage.resolution_30m.ttl",
+		"ui.display_timezone",
+		"version",
+	}
+}
+
 // LookupForLocalAccess returns a NonMaskedSetting by name. Used when a setting
 // is being retrieved for local processing within the cluster and not for
 // reporting; sensitive values are accessible.