Skip to content

Commit

Permalink
server, sql: add VIEWCLUSTERSETTING user privilege
Browse files Browse the repository at this point in the history
Before, only users with `admin` role or `MODIFYCLUSTERSETTING`
permission could view cluster settings.
Now, new role is added to provide users view-only permission
to view cluster settings from SQL shell and in Db Console (in
Advanced debugging > Cluster settings).
This change doesn't change behavior for `MODIFYCLUSTERSETTING`
option, it also allows view and modify cluster settings.

Release note (sql change): new user privileges are added: `VIEWCLUSTERSETTING`
and `NOVIEWCLUSTERSETTING` that allows users to view cluster settings
only.
  • Loading branch information
koorosh committed Feb 7, 2022
1 parent 7b48b61 commit c07ac06
Show file tree
Hide file tree
Showing 9 changed files with 120 additions and 16 deletions.
4 changes: 4 additions & 0 deletions docs/generated/sql/bnf/stmt_block.bnf
Original file line number Diff line number Diff line change
Expand Up @@ -1045,6 +1045,7 @@ unreserved_keyword ::=
| 'NOSQLLOGIN'
| 'NOVIEWACTIVITY'
| 'NOVIEWACTIVITYREDACTED'
| 'NOVIEWCLUSTERSETTING'
| 'NOWAIT'
| 'NULLS'
| 'IGNORE_FOREIGN_KEYS'
Expand Down Expand Up @@ -1205,6 +1206,7 @@ unreserved_keyword ::=
| 'VIEW'
| 'VIEWACTIVITY'
| 'VIEWACTIVITYREDACTED'
| 'VIEWCLUSTERSETTING'
| 'VISIBLE'
| 'VOTERS'
| 'WITHIN'
Expand Down Expand Up @@ -2381,6 +2383,8 @@ role_option ::=
| 'NOMODIFYCLUSTERSETTING'
| 'SQLLOGIN'
| 'NOSQLLOGIN'
| 'VIEWCLUSTERSETTING'
| 'NOVIEWCLUSTERSETTING'
| password_clause
| valid_until_clause

Expand Down
18 changes: 17 additions & 1 deletion pkg/server/admin.go
Original file line number Diff line number Diff line change
Expand Up @@ -1670,7 +1670,7 @@ func (s *adminServer) Settings(
keys = settings.Keys(settings.ForSystemTenant)
}

_, isAdmin, err := s.getUserAndRole(ctx)
user, isAdmin, err := s.getUserAndRole(ctx)
if err != nil {
return nil, err
}
Expand All @@ -1689,6 +1689,22 @@ func (s *adminServer) Settings(
lookupPurpose = settings.LookupForReporting
}

hasView, err := s.hasRoleOption(ctx, user, roleoption.VIEWCLUSTERSETTING)
if err != nil {
return nil, err
}

hasModify, err := s.hasRoleOption(ctx, user, roleoption.MODIFYCLUSTERSETTING)
if err != nil {
return nil, err
}

if !hasModify && !hasView {
return nil, status.Errorf(
codes.PermissionDenied, "this operation requires either %s or %s role options",
roleoption.VIEWCLUSTERSETTING, roleoption.MODIFYCLUSTERSETTING)
}

resp := serverpb.SettingsResponse{KeyValues: make(map[string]serverpb.SettingsResponse_Value)}
for _, k := range keys {
v, ok := settings.Lookup(k, lookupPurpose, settings.ForSystemTenant)
Expand Down
10 changes: 7 additions & 3 deletions pkg/sql/crdb_internal.go
Original file line number Diff line number Diff line change
Expand Up @@ -1460,10 +1460,14 @@ CREATE TABLE crdb_internal.cluster_settings (
if err != nil {
return err
}
if !hasModify {
hasView, err := p.HasRoleOption(ctx, roleoption.VIEWCLUSTERSETTING)
if err != nil {
return err
}
if !hasModify && !hasView {
return pgerror.Newf(pgcode.InsufficientPrivilege,
"only users with the %s privilege are allowed to read "+
"crdb_internal.cluster_settings", roleoption.MODIFYCLUSTERSETTING)
"only users with either %s or %s privileges are allowed to read "+
"crdb_internal.cluster_settings", roleoption.MODIFYCLUSTERSETTING, roleoption.VIEWCLUSTERSETTING)
}
}
for _, k := range settings.Keys(p.ExecCfg().Codec.ForSystemTenant()) {
Expand Down
10 changes: 7 additions & 3 deletions pkg/sql/delegate/show_all_cluster_settings.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,14 @@ func (d *delegator) delegateShowClusterSettingList(
if err != nil {
return nil, err
}
if !hasModify {
hasView, err := d.catalog.HasRoleOption(d.ctx, roleoption.VIEWCLUSTERSETTING)
if err != nil {
return nil, err
}
if !hasModify && !hasView {
return nil, pgerror.Newf(pgcode.InsufficientPrivilege,
"only users with the %s privilege are allowed to SHOW CLUSTER SETTINGS",
roleoption.MODIFYCLUSTERSETTING)
"only users with either %s or %s privileges are allowed to SHOW CLUSTER SETTINGS",
roleoption.MODIFYCLUSTERSETTING, roleoption.VIEWCLUSTERSETTING)
}
if stmt.All {
return parse(
Expand Down
52 changes: 49 additions & 3 deletions pkg/sql/logictest/testdata/logic_test/cluster_settings
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,13 @@ user testuser
statement error only users with the MODIFYCLUSTERSETTING privilege are allowed to set cluster setting 'sql.defaults.default_int_size'
SET CLUSTER SETTING sql.defaults.default_int_size = 4

statement error only users with the MODIFYCLUSTERSETTING privilege are allowed to show cluster setting 'sql.defaults.default_int_size'
statement error only users with either MODIFYCLUSTERSETTING or VIEWCLUSTERSETTING privileges are allowed to show cluster setting 'sql.defaults.default_int_size'
SHOW CLUSTER SETTING sql.defaults.default_int_size

statement error only users with the MODIFYCLUSTERSETTING privilege are allowed to SHOW CLUSTER SETTINGS
statement error only users with either MODIFYCLUSTERSETTING or VIEWCLUSTERSETTING privileges are allowed to SHOW CLUSTER SETTINGS
SHOW CLUSTER SETTINGS

statement error only users with the MODIFYCLUSTERSETTING privilege are allowed to SHOW CLUSTER SETTINGS
statement error only users with either MODIFYCLUSTERSETTING or VIEWCLUSTERSETTING privileges are allowed to SHOW CLUSTER SETTINGS
SHOW ALL CLUSTER SETTINGS

user root
Expand Down Expand Up @@ -77,6 +77,52 @@ SET CLUSTER SETTING sql.defaults.default_int_size = 4

user root

statement ok
ALTER USER testuser NOMODIFYCLUSTERSETTING

statement ok
ALTER USER testuser VIEWCLUSTERSETTING

user testuser

statement error only users with the MODIFYCLUSTERSETTING privilege are allowed to set cluster setting 'sql.defaults.default_int_size'
SET CLUSTER SETTING sql.defaults.default_int_size = 4

query I
SHOW CLUSTER SETTING sql.defaults.default_int_size
----
4

query TT
SELECT variable, value FROM [SHOW CLUSTER SETTINGS]
WHERE variable IN ('sql.defaults.default_int_size')
----
sql.defaults.default_int_size 4

query TT
SELECT variable, value FROM [SHOW ALL CLUSTER SETTINGS]
WHERE variable IN ('sql.defaults.default_int_size')
----
sql.defaults.default_int_size 4

user root

statement ok
ALTER USER testuser NOVIEWCLUSTERSETTING

user testuser

statement error only users with the MODIFYCLUSTERSETTING privilege are allowed to set cluster setting 'sql.defaults.default_int_size'
SET CLUSTER SETTING sql.defaults.default_int_size = 4

statement error only users with either MODIFYCLUSTERSETTING or VIEWCLUSTERSETTING privileges are allowed to SHOW CLUSTER SETTINGS
SHOW CLUSTER SETTINGS

statement error only users with either MODIFYCLUSTERSETTING or VIEWCLUSTERSETTING privileges are allowed to SHOW CLUSTER SETTINGS
SHOW ALL CLUSTER SETTINGS

user root

statement ok
GRANT admin TO testuser

Expand Down
15 changes: 13 additions & 2 deletions pkg/sql/parser/sql.y
Original file line number Diff line number Diff line change
Expand Up @@ -825,7 +825,7 @@ func (u *sqlSymUnion) setVar() *tree.SetVar {
%token <str> NAN NAME NAMES NATURAL NEVER NEW_DB_NAME NEXT NO NOCANCELQUERY NOCONTROLCHANGEFEED
%token <str> NOCONTROLJOB NOCREATEDB NOCREATELOGIN NOCREATEROLE NOLOGIN NOMODIFYCLUSTERSETTING
%token <str> NOSQLLOGIN NO_INDEX_JOIN NO_ZIGZAG_JOIN NO_FULL_SCAN NONE NONVOTERS NORMAL NOT NOTHING NOTNULL
%token <str> NOVIEWACTIVITY NOVIEWACTIVITYREDACTED NOWAIT NULL NULLIF NULLS NUMERIC
%token <str> NOVIEWACTIVITY NOVIEWACTIVITYREDACTED NOVIEWCLUSTERSETTING NOWAIT NULL NULLIF NULLS NUMERIC

%token <str> OF OFF OFFSET OID OIDS OIDVECTOR ON ONLY OPT OPTION OPTIONS OR
%token <str> ORDER ORDINALITY OTHERS OUT OUTER OVER OVERLAPS OVERLAY OWNED OWNER OPERATOR
Expand Down Expand Up @@ -861,7 +861,8 @@ func (u *sqlSymUnion) setVar() *tree.SetVar {
%token <str> UNBOUNDED UNCOMMITTED UNION UNIQUE UNKNOWN UNLOGGED UNSPLIT
%token <str> UPDATE UPSERT UNTIL USE USER USERS USING UUID

%token <str> VALID VALIDATE VALUE VALUES VARBIT VARCHAR VARIADIC VIEW VARYING VIEWACTIVITY VIEWACTIVITYREDACTED VIRTUAL VISIBLE VOTERS
%token <str> VALID VALIDATE VALUE VALUES VARBIT VARCHAR VARIADIC VIEW VARYING VIEWACTIVITY VIEWACTIVITYREDACTED
%token <str> VIEWCLUSTERSETTING VIRTUAL VISIBLE VOTERS

%token <str> WHEN WHERE WINDOW WITH WITHIN WITHOUT WORK WRITE

Expand Down Expand Up @@ -7824,6 +7825,14 @@ role_option:
{
$$.val = tree.KVOption{Key: tree.Name($1), Value: nil}
}
| VIEWCLUSTERSETTING
{
$$.val = tree.KVOption{Key: tree.Name($1), Value: nil}
}
| NOVIEWCLUSTERSETTING
{
$$.val = tree.KVOption{Key: tree.Name($1), Value: nil}
}
| password_clause
| valid_until_clause

Expand Down Expand Up @@ -13560,6 +13569,7 @@ unreserved_keyword:
| NOSQLLOGIN
| NOVIEWACTIVITY
| NOVIEWACTIVITYREDACTED
| NOVIEWCLUSTERSETTING
| NOWAIT
| NULLS
| IGNORE_FOREIGN_KEYS
Expand Down Expand Up @@ -13720,6 +13730,7 @@ unreserved_keyword:
| VIEW
| VIEWACTIVITY
| VIEWACTIVITYREDACTED
| VIEWCLUSTERSETTING
| VISIBLE
| VOTERS
| WITHIN
Expand Down
6 changes: 4 additions & 2 deletions pkg/sql/roleoption/option_string.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 9 additions & 1 deletion pkg/sql/roleoption/role_option.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ const (
NOVIEWACTIVITYREDACTED
SQLLOGIN
NOSQLLOGIN
VIEWCLUSTERSETTING
NOVIEWCLUSTERSETTING
)

// toSQLStmts is a map of Kind -> SQL statement string for applying the
Expand Down Expand Up @@ -88,6 +90,8 @@ var toSQLStmts = map[Option]string{
NOSQLLOGIN: `UPSERT INTO system.role_options (username, option) VALUES ($1, 'NOSQLLOGIN')`,
VIEWACTIVITYREDACTED: `UPSERT INTO system.role_options (username, option) VALUES ($1, 'VIEWACTIVITYREDACTED')`,
NOVIEWACTIVITYREDACTED: `DELETE FROM system.role_options WHERE username = $1 AND option = 'VIEWACTIVITYREDACTED'`,
VIEWCLUSTERSETTING: `UPSERT INTO system.role_options (username, option) VALUES ($1, 'VIEWCLUSTERSETTING')`,
NOVIEWCLUSTERSETTING: `DELETE FROM system.role_options WHERE username = $1 AND option = 'VIEWCLUSTERSETTING'`,
}

// Mask returns the bitmask for a given role option.
Expand Down Expand Up @@ -122,6 +126,8 @@ var ByName = map[string]Option{
"NOVIEWACTIVITYREDACTED": NOVIEWACTIVITYREDACTED,
"SQLLOGIN": SQLLOGIN,
"NOSQLLOGIN": NOSQLLOGIN,
"VIEWCLUSTERSETTING": VIEWCLUSTERSETTING,
"NOVIEWCLUSTERSETTING": NOVIEWCLUSTERSETTING,
}

// ToOption takes a string and returns the corresponding Option.
Expand Down Expand Up @@ -229,7 +235,9 @@ func (rol List) CheckRoleOptionConflicts() error {
(roleOptionBits&VIEWACTIVITYREDACTED.Mask() != 0 &&
roleOptionBits&NOVIEWACTIVITYREDACTED.Mask() != 0) ||
(roleOptionBits&SQLLOGIN.Mask() != 0 &&
roleOptionBits&NOSQLLOGIN.Mask() != 0) {
roleOptionBits&NOSQLLOGIN.Mask() != 0) ||
(roleOptionBits&VIEWCLUSTERSETTING.Mask() != 0 &&
roleOptionBits&NOVIEWCLUSTERSETTING.Mask() != 0) {
return pgerror.Newf(pgcode.Syntax, "conflicting role options")
}
return nil
Expand Down
11 changes: 10 additions & 1 deletion pkg/sql/set_cluster_setting.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,11 +67,20 @@ func checkPrivilegesForSetting(ctx context.Context, p *planner, name string, act
if err != nil {
return err
}
if !hasModify {
if action == "set" && !hasModify {
return pgerror.Newf(pgcode.InsufficientPrivilege,
"only users with the %s privilege are allowed to %s cluster setting '%s'",
roleoption.MODIFYCLUSTERSETTING, action, name)
}
hasView, err := p.HasRoleOption(ctx, roleoption.VIEWCLUSTERSETTING)
if err != nil {
return err
}
if action == "show" && !(hasModify || hasView) {
return pgerror.Newf(pgcode.InsufficientPrivilege,
"only users with either %s or %s privileges are allowed to %s cluster setting '%s'",
roleoption.MODIFYCLUSTERSETTING, roleoption.VIEWCLUSTERSETTING, action, name)
}
return nil
}

Expand Down

0 comments on commit c07ac06

Please sign in to comment.