Skip to content

Commit

Permalink
server: allow users with VIEWACTIVITY to get console settings
Browse files Browse the repository at this point in the history
Previously, only users with `ADMIN`, `VIEWCLUSTERSETTING` or
`MODIFYCLUSTERSETTING` could get the settings on the
`_admin/v1/settings`. This API was used by the Console and then
a few places retrieved that information.
Users without those permissions would not be able to see the
values and some functionalities where not working as expected,
for example timezone was showing as UTC even when the cluster
setting `ui.display_timezone` was set to other value.

This commits modifies that endpoint (and that endpoint only)
to return just the cluster settings that are not sensitive and
that are required by the console to have all functionalities
working.

The list of cluster settings:
"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"

Part Of cockroachdb#108373

Release note (bug fix): Users with VIEWACTIVITY can now view
correct values for timezone.
  • Loading branch information
maryliag committed Aug 16, 2023
1 parent 3b2319d commit 94d2ec4
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 8 deletions.
1 change: 1 addition & 0 deletions pkg/server/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
43 changes: 35 additions & 8 deletions pkg/server/admin.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand All @@ -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)
}
}
}
}

Expand Down Expand Up @@ -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 {
Expand Down
23 changes: 23 additions & 0 deletions pkg/settings/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down

0 comments on commit 94d2ec4

Please sign in to comment.