Skip to content

Commit

Permalink
Merge #40879
Browse files Browse the repository at this point in the history
40879: sql: check privileges in CONFIGURE ZONE commands r=solongordon a=solongordon

Previously, any user could apply zone configurations to any object. This
commit enforces that users must have appropriate privileges to run a
CONFIGURE ZONE command. For system ranges, the system database, and
tables in that database, the user must be an admin. For other databases
and tables, the user must have CREATE privileges on the object.

Release note (backward-incompatible change): CONFIGURE ZONE commands
now fail if the user does not have sufficient privileges. If the target
is a system range, the "system" database, or a table in that database,
the user must be an admin. For all other databases and tables, the user
must have the CREATE privilege on the target database or table.

Note that this change may be backward-incompatible for users who run
scripted CONFIGURE ZONE commands via a user with restricted permissions.
To add the necessary permissions, use the GRANT command via an admin
user. To grant the admin role to a user, run `GRANT admin TO <user>`. To
grant the CREATE privilege on a database or table, run `GRANT CREATE ON
[DATABASE | TABLE] <name> TO <user>`.

Release justification: Fix for high-priority bug in existing
functionality.

Fixes #40693

Co-authored-by: Solon Gordon <solon@cockroachlabs.com>
  • Loading branch information
craig[bot] and solongordon committed Sep 23, 2019
2 parents 29f5d18 + 9e04472 commit 9f7e253
Show file tree
Hide file tree
Showing 2 changed files with 100 additions and 3 deletions.
65 changes: 65 additions & 0 deletions pkg/ccl/logictestccl/testdata/logic_test/zone
Original file line number Diff line number Diff line change
Expand Up @@ -697,3 +697,68 @@ t40417 CREATE TABLE t40417 (
);
ALTER PARTITION p1 OF INDEX "my database".public.t40417@primary CONFIGURE ZONE USING
num_replicas = 1

subtest authorization

statement ok
CREATE DATABASE auth;
CREATE TABLE auth.t (x INT PRIMARY KEY) PARTITION BY LIST (x) (
PARTITION p VALUES IN (1)
);
CREATE INDEX x ON auth.t (x) PARTITION BY LIST (x) (
PARTITION p VALUES IN (1)
)

user testuser

# User should have no CONFIGURE ZONE abilities by default.
statement error only users with the admin role are allowed to alter system ranges
ALTER RANGE default CONFIGURE ZONE USING num_replicas = 3

statement error pq: user testuser does not have CREATE privilege on database auth
ALTER DATABASE auth CONFIGURE ZONE USING num_replicas = 3

statement error pq: only users with the admin role are allowed to alter system tables
ALTER TABLE system.jobs CONFIGURE ZONE USING num_replicas = 3

statement error user testuser does not have CREATE privilege on relation t
ALTER TABLE auth.t CONFIGURE ZONE USING num_replicas = 3

statement error user testuser does not have CREATE privilege on relation t
ALTER PARTITION p OF TABLE auth.t CONFIGURE ZONE USING num_replicas = 3

statement error user testuser does not have CREATE privilege on relation t
ALTER PARTITION p OF INDEX auth.t@x CONFIGURE ZONE USING num_replicas = 3

# Granting CREATE on databases and tables should allow CONFIGURE ZONE on those
# objects.
user root

statement ok
GRANT CREATE ON DATABASE auth TO testuser

statement ok
GRANT CREATE ON TABLE auth.t TO testuser

user testuser

statement ok
ALTER DATABASE auth CONFIGURE ZONE USING num_replicas = 3

statement ok
ALTER TABLE auth.t CONFIGURE ZONE USING num_replicas = 3

# Granting the admin role should allow configuring zones on system tables and
# ranges.
user root

statement ok
GRANT admin TO testuser

user testuser

statement ok
ALTER RANGE default CONFIGURE ZONE USING num_replicas = 3

statement ok
ALTER TABLE system.jobs CONFIGURE ZONE USING num_replicas = 3
38 changes: 35 additions & 3 deletions pkg/sql/set_zone_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/server/serverpb"
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
"github.com/cockroachdb/cockroach/pkg/sql/privilege"
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
"github.com/cockroachdb/cockroach/pkg/sql/sqlbase"
"github.com/cockroachdb/cockroach/pkg/sql/sqltelemetry"
Expand Down Expand Up @@ -94,6 +95,10 @@ func loadYAML(dst interface{}, yamlString string) {
}

func (p *planner) SetZoneConfig(ctx context.Context, n *tree.SetZoneConfig) (planNode, error) {
if err := checkPrivilegeForSetZoneConfig(ctx, p, n.ZoneSpecifier); err != nil {
return nil, err
}

var yamlConfig tree.TypedExpr

if n.YAMLConfig != nil {
Expand Down Expand Up @@ -159,6 +164,36 @@ func (p *planner) SetZoneConfig(ctx context.Context, n *tree.SetZoneConfig) (pla
}, nil
}

func checkPrivilegeForSetZoneConfig(ctx context.Context, p *planner, zs tree.ZoneSpecifier) error {
// For system ranges, the system database, or system tables, the user must be
// an admin. Otherwise we require CREATE privileges on the database or table
// in question.
if zs.NamedZone != "" {
return p.RequireAdminRole(ctx, "alter system ranges")
}
if zs.Database != "" {
if zs.Database == "system" {
return p.RequireAdminRole(ctx, "alter the system database")
}
dbDesc, err := p.ResolveUncachedDatabaseByName(ctx, string(zs.Database), true)
if err != nil {
return err
}
return p.CheckPrivilege(ctx, dbDesc, privilege.CREATE)
}
tableDesc, err := p.resolveTableForZone(ctx, &zs)
if err != nil {
if zs.TargetsIndex() && zs.TableOrIndex.Table.TableName == "" {
err = errors.WithHint(err, "try specifying the index as <tablename>@<indexname>")
}
return err
}
if tableDesc.ParentID == keys.SystemDatabaseID {
return p.RequireAdminRole(ctx, "alter system tables")
}
return p.CheckPrivilege(ctx, tableDesc, privilege.CREATE)
}

// setZoneConfigRun contains the run-time state of setZoneConfigNode during local execution.
type setZoneConfigRun struct {
numAffected int
Expand Down Expand Up @@ -241,9 +276,6 @@ func (n *setZoneConfigNode) startExec(params runParams) error {
// descriptor.
table, err := params.p.resolveTableForZone(params.ctx, &n.zoneSpecifier)
if err != nil {
if n.zoneSpecifier.TargetsIndex() && n.zoneSpecifier.TableOrIndex.Table.TableName == "" {
return errors.WithHint(err, "try specifying the index as <tablename>@<indexname>")
}
return err
}

Expand Down

0 comments on commit 9f7e253

Please sign in to comment.