diff --git a/pkg/ccl/backupccl/alter_backup_schedule.go b/pkg/ccl/backupccl/alter_backup_schedule.go index ff0c5e5f87c3..bcd236ea0a6c 100644 --- a/pkg/ccl/backupccl/alter_backup_schedule.go +++ b/pkg/ccl/backupccl/alter_backup_schedule.go @@ -299,8 +299,11 @@ func processScheduleOptions( // NB: as of 20.2, schedule creation requires admin so this is duplicative // but in the future we might relax so you can schedule anything that you // can backup, but then this cluster-wide metric should be admin-only. - if err := p.RequireAdminRole(ctx, optUpdatesLastBackupMetric); err != nil { - return pgerror.Wrap(err, pgcode.InsufficientPrivilege, "") + if hasAdmin, err := p.HasAdminRole(ctx); err != nil { + return err + } else if !hasAdmin { + return pgerror.Newf(pgcode.InsufficientPrivilege, + "only users with the admin role are allowed to change %s", optUpdatesLastBackupMetric) } updatesLastBackupMetric, err := strconv.ParseBool(v) diff --git a/pkg/ccl/backupccl/create_scheduled_backup.go b/pkg/ccl/backupccl/create_scheduled_backup.go index 78d40a601d4f..c8512ec75b90 100644 --- a/pkg/ccl/backupccl/create_scheduled_backup.go +++ b/pkg/ccl/backupccl/create_scheduled_backup.go @@ -27,6 +27,8 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql" "github.com/cockroachdb/cockroach/pkg/sql/catalog/colinfo" "github.com/cockroachdb/cockroach/pkg/sql/exprutil" + "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode" + "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror" "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgnotice" "github.com/cockroachdb/cockroach/pkg/sql/sem/eval" "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" @@ -286,8 +288,11 @@ func doCreateBackupSchedules( // NB: as of 20.2, schedule creation requires admin so this is duplicative // but in the future we might relax so you can schedule anything that you // can backup, but then this cluster-wide metric should be admin-only. - if err := p.RequireAdminRole(ctx, optUpdatesLastBackupMetric); err != nil { + if hasAdmin, err := p.HasAdminRole(ctx); err != nil { return err + } else if !hasAdmin { + return pgerror.Newf(pgcode.InsufficientPrivilege, + "only users with the admin role are allowed to change %s", optUpdatesLastBackupMetric) } } diff --git a/pkg/ccl/backupccl/testdata/backup-restore/alter-schedule/schedule-options b/pkg/ccl/backupccl/testdata/backup-restore/alter-schedule/schedule-options index 2436e568d73f..b7aee6c0bc1c 100644 --- a/pkg/ccl/backupccl/testdata/backup-restore/alter-schedule/schedule-options +++ b/pkg/ccl/backupccl/testdata/backup-restore/alter-schedule/schedule-options @@ -119,7 +119,7 @@ let $fullID $incID with schedules as (show schedules) select id from schedules where label='datatest3' order by command->>'backup_type' asc; ---- -exec-sql user=testuser expect-error-regex=(only users with the admin role are allowed to updates_cluster_last_backup_time_metric) +exec-sql user=testuser expect-error-regex=(only users with the admin role are allowed to change updates_cluster_last_backup_time_metric) alter backup schedule $fullID set schedule option updates_cluster_last_backup_time_metric = '1'; ---- regex matches error diff --git a/pkg/ccl/logictestccl/testdata/logic_test/crdb_internal b/pkg/ccl/logictestccl/testdata/logic_test/crdb_internal index 80ec885db1ce..fb913e2a622b 100644 --- a/pkg/ccl/logictestccl/testdata/logic_test/crdb_internal +++ b/pkg/ccl/logictestccl/testdata/logic_test/crdb_internal @@ -181,7 +181,7 @@ subtest node_tenant_capabilities_cache user testuser -statement error only users with the admin role are allowed to node_tenant_capabilities_cache +statement error user testuser does not have VIEWCLUSTERMETADATA system privilege SELECT * FROM crdb_internal.node_tenant_capabilities_cache user root diff --git a/pkg/ccl/logictestccl/testdata/logic_test/crdb_internal_tenant b/pkg/ccl/logictestccl/testdata/logic_test/crdb_internal_tenant index 1837f2269b02..c8126cd10bb8 100644 --- a/pkg/ccl/logictestccl/testdata/logic_test/crdb_internal_tenant +++ b/pkg/ccl/logictestccl/testdata/logic_test/crdb_internal_tenant @@ -460,28 +460,28 @@ select crdb_internal.set_vmodule('') query error insufficient privilege select crdb_internal.get_vmodule() -query error pq: only users with the admin role are allowed to access the node runtime information +query error user testuser does not have VIEWCLUSTERMETADATA system privilege select * from crdb_internal.node_runtime_info query error pq: only users with the VIEWACTIVITY or VIEWACTIVITYREDACTED or ZONECONFIG privilege or the admin role can read crdb_internal.ranges_no_leases select * from crdb_internal.ranges -query error pq: only users with the admin role are allowed to read crdb_internal.gossip_nodes +query error user testuser does not have VIEWCLUSTERMETADATA system privilege select * from crdb_internal.gossip_nodes -query error pq: only users with the admin role are allowed to read crdb_internal.gossip_liveness +query error user testuser does not have VIEWCLUSTERMETADATA system privilege select * from crdb_internal.gossip_liveness -query error pq: only users with the admin role are allowed to read crdb_internal.node_metrics +query error user testuser does not have VIEWCLUSTERMETADATA system privilege select * from crdb_internal.node_metrics -query error pq: only users with the admin role are allowed to read crdb_internal.kv_node_status +query error user testuser does not have VIEWCLUSTERMETADATA system privilege select * from crdb_internal.kv_node_status -query error pq: only users with the admin role are allowed to read crdb_internal.kv_store_status +query error user testuser does not have VIEWCLUSTERMETADATA system privilege select * from crdb_internal.kv_store_status -query error pq: only users with the admin role are allowed to read crdb_internal.gossip_alerts +query error user testuser does not have VIEWCLUSTERMETADATA system privilege select * from crdb_internal.gossip_alerts # Anyone can see the executable version. @@ -637,7 +637,7 @@ subtest node_tenant_capabilities_cache user testuser -statement error only users with the admin role are allowed to node_tenant_capabilities_cache +statement error user testuser does not have VIEWCLUSTERMETADATA system privilege SELECT * FROM crdb_internal.node_tenant_capabilities_cache user root diff --git a/pkg/ccl/logictestccl/testdata/logic_test/tenant_usage b/pkg/ccl/logictestccl/testdata/logic_test/tenant_usage index a9460790e9b6..df423a2c6074 100644 --- a/pkg/ccl/logictestccl/testdata/logic_test/tenant_usage +++ b/pkg/ccl/logictestccl/testdata/logic_test/tenant_usage @@ -34,5 +34,5 @@ SELECT crdb_internal.update_tenant_resource_limits('apptenant', 1000, 100, 0, no user testuser -statement error only users with the admin role are allowed to update tenant resource limits +statement error crdb_internal.update_tenant_resource_limits\(\): user testuser does not have REPAIRCLUSTERMETADATA system privilege SELECT crdb_internal.update_tenant_resource_limits(5, 1000, 100, 0, now(), 0) diff --git a/pkg/ccl/logictestccl/testdata/logic_test/zone b/pkg/ccl/logictestccl/testdata/logic_test/zone index cba2788f6275..08fa5fe6e74d 100644 --- a/pkg/ccl/logictestccl/testdata/logic_test/zone +++ b/pkg/ccl/logictestccl/testdata/logic_test/zone @@ -785,13 +785,13 @@ CREATE INDEX x ON auth.t (x) PARTITION BY LIST (x) ( 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 +statement error user testuser does not have REPAIRCLUSTERMETADATA system privilege ALTER RANGE default CONFIGURE ZONE USING num_replicas = 3 statement error pq: user testuser does not have ZONECONFIG or 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 +statement error user testuser does not have REPAIRCLUSTERMETADATA system privilege ALTER TABLE system.jobs CONFIGURE ZONE USING num_replicas = 3 statement error pq: user testuser does not have ZONECONFIG or CREATE privilege on relation t diff --git a/pkg/sql/alter_default_privileges.go b/pkg/sql/alter_default_privileges.go index 6fbfebfe0b26..d0b7f02f0dc0 100644 --- a/pkg/sql/alter_default_privileges.go +++ b/pkg/sql/alter_default_privileges.go @@ -140,8 +140,11 @@ func (n *alterDefaultPrivilegesNode) startExec(params runParams) error { } if n.n.ForAllRoles { - if err := params.p.RequireAdminRole(params.ctx, "ALTER DEFAULT PRIVILEGES"); err != nil { + if hasAdmin, err := params.p.HasAdminRole(params.ctx); err != nil { return err + } else if !hasAdmin { + return pgerror.Newf(pgcode.InsufficientPrivilege, + "only users with the admin role are allowed to ALTER DEFAULT PRIVILEGES FOR ALL ROLES") } } else { // You can change default privileges only for objects that will be created diff --git a/pkg/sql/alter_role.go b/pkg/sql/alter_role.go index b9e11a0db192..b4fadc482759 100644 --- a/pkg/sql/alter_role.go +++ b/pkg/sql/alter_role.go @@ -32,6 +32,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/sessioninit" "github.com/cockroachdb/cockroach/pkg/sql/sqlerrors" "github.com/cockroachdb/cockroach/pkg/sql/sqltelemetry" + "github.com/cockroachdb/cockroach/pkg/sql/syntheticprivilege" "github.com/cockroachdb/cockroach/pkg/sql/types" "github.com/cockroachdb/cockroach/pkg/util/log/eventpb" ) @@ -215,8 +216,11 @@ func (n *alterRoleNode) startExec(params runParams) error { return err } if isAdmin { - if err := params.p.RequireAdminRole(params.ctx, "ALTER ROLE admin"); err != nil { + if hasAdmin, err := params.p.HasAdminRole(params.ctx); err != nil { return err + } else if !hasAdmin { + return pgerror.Newf(pgcode.InsufficientPrivilege, + "only users with the admin role are allowed to alter another admin") } } @@ -287,8 +291,11 @@ func (p *planner) AlterRoleSet(ctx context.Context, n *tree.AlterRoleSet) (planN // modifying their own defaults unless they have CREATEROLE. This is analogous // to our restriction that prevents a user from modifying their own password. if n.AllRoles { - if err := p.RequireAdminRole(ctx, "ALTER ROLE ALL"); err != nil { + if hasAdmin, err := p.HasAdminRole(ctx); err != nil { return nil, err + } else if !hasAdmin { + return nil, pgerror.Newf(pgcode.InsufficientPrivilege, + "only users with the admin role are allowed to ALTER ROLE ALL ... SET") } } else { canAlterRoleSet, err := p.HasGlobalPrivilegeOrRoleOption(ctx, privilege.CREATEROLE) @@ -607,7 +614,17 @@ func (n *alterRoleSetNode) getRoleName( return false, username.SQLUsername{}, err } if isAdmin { - if err := params.p.RequireAdminRole(params.ctx, "ALTER ROLE admin"); err != nil { + if hasAdmin, err := params.p.HasAdminRole(params.ctx); err != nil { + return false, username.SQLUsername{}, err + } else if !hasAdmin { + return false, username.SQLUsername{}, pgerror.Newf(pgcode.InsufficientPrivilege, + "only users with the admin role are allowed to alter another admin") + } + + // Note that admins implicitly have the REPAIRCLUSTERMETADATA privilege. + if err := params.p.CheckPrivilege( + params.ctx, syntheticprivilege.GlobalPrivilegeObject, privilege.REPAIRCLUSTERMETADATA, + ); err != nil { return false, username.SQLUsername{}, err } } diff --git a/pkg/sql/authorization.go b/pkg/sql/authorization.go index 8919bee20ad1..bc7f1fe676e9 100644 --- a/pkg/sql/authorization.go +++ b/pkg/sql/authorization.go @@ -107,11 +107,6 @@ type AuthorizationAccessor interface { // HasAdminRole checks if the current session's user has admin role. HasAdminRole(ctx context.Context) (bool, error) - // RequireAdminRole is a wrapper on top of HasAdminRole. - // It errors if HasAdminRole errors or if the user isn't a super-user. - // Includes the named action in the error message. - RequireAdminRole(ctx context.Context, action string) error - // MemberOfWithAdminOption looks up all the roles (direct and indirect) that 'member' is a member // of and returns a map of role -> isAdmin. MemberOfWithAdminOption(ctx context.Context, member username.SQLUsername) (map[username.SQLUsername]bool, error) @@ -493,22 +488,6 @@ func (p *planner) HasAdminRole(ctx context.Context) (bool, error) { return p.UserHasAdminRole(ctx, p.User()) } -// RequireAdminRole implements the AuthorizationAccessor interface. -// Requires a valid transaction to be open. -func (p *planner) RequireAdminRole(ctx context.Context, action string) error { - ok, err := p.HasAdminRole(ctx) - - if err != nil { - return err - } - if !ok { - // raise error if user is not a super-user - return pgerror.Newf(pgcode.InsufficientPrivilege, - "only users with the admin role are allowed to %s", action) - } - return nil -} - // MemberOfWithAdminOption is a wrapper around the MemberOfWithAdminOption // method. func (p *planner) MemberOfWithAdminOption( diff --git a/pkg/sql/copy_file_upload.go b/pkg/sql/copy_file_upload.go index 7dc12ee2f98b..2eaa978ec7c7 100644 --- a/pkg/sql/copy_file_upload.go +++ b/pkg/sql/copy_file_upload.go @@ -19,6 +19,8 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/catalog/colinfo" "github.com/cockroachdb/cockroach/pkg/sql/lexbase" + "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode" + "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror" "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgwirebase" "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" "github.com/cockroachdb/cockroach/pkg/sql/types" @@ -107,8 +109,11 @@ func newFileUploadMachine( c.parsingEvalCtx = c.p.EvalContext() if n.Table.Table() == NodelocalFileUploadTable { - if err := c.p.RequireAdminRole(ctx, "upload to nodelocal"); err != nil { + if hasAdmin, err := p.HasAdminRole(ctx); err != nil { return nil, err + } else if !hasAdmin { + return nil, pgerror.Newf(pgcode.InsufficientPrivilege, + "only users with the admin role are allowed to upload to nodelocal") } } diff --git a/pkg/sql/crdb_internal.go b/pkg/sql/crdb_internal.go index b77ba4ad8571..95c51378174f 100644 --- a/pkg/sql/crdb_internal.go +++ b/pkg/sql/crdb_internal.go @@ -82,6 +82,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/sqlstats" "github.com/cockroachdb/cockroach/pkg/sql/sqlstats/persistedsqlstats/sqlstatsutil" "github.com/cockroachdb/cockroach/pkg/sql/sqlstats/sslocal" + "github.com/cockroachdb/cockroach/pkg/sql/syntheticprivilege" "github.com/cockroachdb/cockroach/pkg/sql/types" "github.com/cockroachdb/cockroach/pkg/sql/vtable" "github.com/cockroachdb/cockroach/pkg/util/duration" @@ -256,7 +257,7 @@ CREATE TABLE crdb_internal.node_runtime_info ( value STRING NOT NULL )`, populate: func(ctx context.Context, p *planner, _ catalog.DatabaseDescriptor, addRow func(...tree.Datum) error) error { - if err := p.RequireAdminRole(ctx, "access the node runtime information"); err != nil { + if err := p.CheckPrivilege(ctx, syntheticprivilege.GlobalPrivilegeObject, privilege.VIEWCLUSTERMETADATA); err != nil { return err } @@ -1831,7 +1832,7 @@ CREATE TABLE crdb_internal.node_txn_stats ( implicit_count INT NOT NULL )`, populate: func(ctx context.Context, p *planner, _ catalog.DatabaseDescriptor, addRow func(...tree.Datum) error) error { - if err := p.RequireAdminRole(ctx, "access application statistics"); err != nil { + if err := p.CheckPrivilege(ctx, syntheticprivilege.GlobalPrivilegeObject, privilege.VIEWCLUSTERMETADATA); err != nil { return err } @@ -1987,14 +1988,12 @@ CREATE TABLE crdb_internal.node_inflight_trace_spans ( operation STRING NULL -- The span's operation. )`, populate: func(ctx context.Context, p *planner, _ catalog.DatabaseDescriptor, addRow func(...tree.Datum) error) error { - hasAdmin, err := p.HasAdminRole(ctx) - if err != nil { + if err := p.CheckPrivilege( + ctx, syntheticprivilege.GlobalPrivilegeObject, privilege.VIEWCLUSTERMETADATA, + ); err != nil { return err } - if !hasAdmin { - return pgerror.Newf(pgcode.InsufficientPrivilege, - "only users with the admin role are allowed to read crdb_internal.node_inflight_trace_spans") - } + return p.ExecCfg().AmbientCtx.Tracer.VisitSpans(func(span tracing.RegistrySpan) error { trace := span.GetFullRecording(tracingpb.RecordingVerbose) for _, rec := range trace.Flatten() { @@ -2973,7 +2972,7 @@ var crdbInternalLocalMetricsTable = virtualSchemaTable{ value FLOAT NOT NULL -- value of the metric )`, populate: func(ctx context.Context, p *planner, _ catalog.DatabaseDescriptor, addRow func(...tree.Datum) error) error { - if err := p.RequireAdminRole(ctx, "read crdb_internal.node_metrics"); err != nil { + if err := p.CheckPrivilege(ctx, syntheticprivilege.GlobalPrivilegeObject, privilege.VIEWCLUSTERMETADATA); err != nil { return err } @@ -4554,7 +4553,7 @@ CREATE TABLE crdb_internal.gossip_nodes ( ) `, populate: func(ctx context.Context, p *planner, _ catalog.DatabaseDescriptor, addRow func(...tree.Datum) error) error { - if err := p.RequireAdminRole(ctx, "read crdb_internal.gossip_nodes"); err != nil { + if err := p.CheckPrivilege(ctx, syntheticprivilege.GlobalPrivilegeObject, privilege.VIEWCLUSTERMETADATA); err != nil { return err } @@ -4672,7 +4671,7 @@ CREATE TABLE crdb_internal.kv_node_liveness ( ) `, populate: func(ctx context.Context, p *planner, _ catalog.DatabaseDescriptor, addRow func(...tree.Datum) error) error { - if err := p.RequireAdminRole(ctx, "read crdb_internal.node_liveness"); err != nil { + if err := p.CheckPrivilege(ctx, syntheticprivilege.GlobalPrivilegeObject, privilege.VIEWCLUSTERMETADATA); err != nil { return err } @@ -4731,7 +4730,7 @@ CREATE TABLE crdb_internal.gossip_liveness ( // which is highly available. DO NOT CALL functions which require the // cluster to be healthy, such as NodesStatusServer.ListNodesInternal(). - if err := p.RequireAdminRole(ctx, "read crdb_internal.gossip_liveness"); err != nil { + if err := p.CheckPrivilege(ctx, syntheticprivilege.GlobalPrivilegeObject, privilege.VIEWCLUSTERMETADATA); err != nil { return err } @@ -4807,7 +4806,7 @@ CREATE TABLE crdb_internal.gossip_alerts ( ) `, populate: func(ctx context.Context, p *planner, _ catalog.DatabaseDescriptor, addRow func(...tree.Datum) error) error { - if err := p.RequireAdminRole(ctx, "read crdb_internal.gossip_alerts"); err != nil { + if err := p.CheckPrivilege(ctx, syntheticprivilege.GlobalPrivilegeObject, privilege.VIEWCLUSTERMETADATA); err != nil { return err } @@ -5128,7 +5127,7 @@ CREATE TABLE crdb_internal.kv_node_status ( ) `, populate: func(ctx context.Context, p *planner, _ catalog.DatabaseDescriptor, addRow func(...tree.Datum) error) error { - if err := p.RequireAdminRole(ctx, "read crdb_internal.kv_node_status"); err != nil { + if err := p.CheckPrivilege(ctx, syntheticprivilege.GlobalPrivilegeObject, privilege.VIEWCLUSTERMETADATA); err != nil { return err } ss, err := p.extendedEvalCtx.NodesStatusServer.OptionalNodesStatusServer( @@ -5242,7 +5241,7 @@ CREATE TABLE crdb_internal.kv_store_status ( ) `, populate: func(ctx context.Context, p *planner, _ catalog.DatabaseDescriptor, addRow func(...tree.Datum) error) error { - if err := p.RequireAdminRole(ctx, "read crdb_internal.kv_store_status"); err != nil { + if err := p.CheckPrivilege(ctx, syntheticprivilege.GlobalPrivilegeObject, privilege.VIEWCLUSTERMETADATA); err != nil { return err } ss, err := p.ExecCfg().NodesStatusServer.OptionalNodesStatusServer( @@ -8130,7 +8129,7 @@ CREATE TABLE crdb_internal.node_tenant_capabilities_cache ( );`, populate: func(ctx context.Context, p *planner, _ catalog.DatabaseDescriptor, addRow func(...tree.Datum) error) error { const op = "node_tenant_capabilities_cache" - if err := p.RequireAdminRole(ctx, op); err != nil { + if err := p.CheckPrivilege(ctx, syntheticprivilege.GlobalPrivilegeObject, privilege.VIEWCLUSTERMETADATA); err != nil { return err } tenantCapabilitiesReader, err := p.ExecCfg().TenantCapabilitiesReader.Get(op) diff --git a/pkg/sql/delegate/show_all_cluster_settings.go b/pkg/sql/delegate/show_all_cluster_settings.go index 6024648c9dc0..162eb5e0402c 100644 --- a/pkg/sql/delegate/show_all_cluster_settings.go +++ b/pkg/sql/delegate/show_all_cluster_settings.go @@ -95,10 +95,7 @@ func (d *delegator) delegateShowTenantClusterSettingList( // privileged operation than viewing local cluster settings. So we // shouldn't be allowing with just the role option // VIEWCLUSTERSETTINGS. - // - // TODO(knz): Using admin authz for now; we may want to introduce a - // more specific role option later. - if err := d.catalog.RequireAdminRole(d.ctx, "show a tenant cluster setting"); err != nil { + if err := d.catalog.CheckPrivilege(d.ctx, syntheticprivilege.GlobalPrivilegeObject, privilege.VIEWCLUSTERMETADATA); err != nil { return nil, err } diff --git a/pkg/sql/gossip.go b/pkg/sql/gossip.go index c4a16243678c..5940e6aee828 100644 --- a/pkg/sql/gossip.go +++ b/pkg/sql/gossip.go @@ -10,7 +10,12 @@ package sql -import "context" +import ( + "context" + + "github.com/cockroachdb/cockroach/pkg/sql/privilege" + "github.com/cockroachdb/cockroach/pkg/sql/syntheticprivilege" +) // TryClearGossipInfo implements the tree.GossipOperator interface. func (p *planner) TryClearGossipInfo(ctx context.Context, key string) (bool, error) { @@ -18,7 +23,7 @@ func (p *planner) TryClearGossipInfo(ctx context.Context, key string) (bool, err if err != nil { return false, err } - if err := p.RequireAdminRole(ctx, "try clear gossip info"); err != nil { + if err := p.CheckPrivilege(ctx, syntheticprivilege.GlobalPrivilegeObject, privilege.REPAIRCLUSTERMETADATA); err != nil { return false, err } return g.TryClearInfo(key) diff --git a/pkg/sql/logictest/testdata/logic_test/alter_default_privileges_for_all_roles b/pkg/sql/logictest/testdata/logic_test/alter_default_privileges_for_all_roles index f71a1fd5a204..0c40947dee52 100644 --- a/pkg/sql/logictest/testdata/logic_test/alter_default_privileges_for_all_roles +++ b/pkg/sql/logictest/testdata/logic_test/alter_default_privileges_for_all_roles @@ -112,7 +112,7 @@ test public t7 testuser2 ALL false user testuser # Must be an admin to alter default privileges for all roles. -statement error pq: only users with the admin role are allowed to ALTER DEFAULT PRIVILEGES +statement error pq: only users with the admin role are allowed to ALTER DEFAULT PRIVILEGES FOR ALL ROLES ALTER DEFAULT PRIVILEGES FOR ALL ROLES GRANT SELECT ON TABLES TO testuser user root diff --git a/pkg/sql/logictest/testdata/logic_test/alter_role_set b/pkg/sql/logictest/testdata/logic_test/alter_role_set index 8b0724e27fc3..ca79d6342952 100644 --- a/pkg/sql/logictest/testdata/logic_test/alter_role_set +++ b/pkg/sql/logictest/testdata/logic_test/alter_role_set @@ -218,7 +218,7 @@ statement error only users with the admin role are allowed to ALTER ROLE ALL ALTER ROLE ALL IN DATABASE test_set_db RESET application_name # Verify that testuser can't edit an ADMIN, even after getting CREATEROLE. -statement error only users with the admin role are allowed to ALTER ROLE admin +statement error only users with the admin role are allowed to alter another admin ALTER ROLE other_admin SET application_name = 'abc' user root diff --git a/pkg/sql/logictest/testdata/logic_test/builtin_function b/pkg/sql/logictest/testdata/logic_test/builtin_function index daaeddf2b748..180d508e64cb 100644 --- a/pkg/sql/logictest/testdata/logic_test/builtin_function +++ b/pkg/sql/logictest/testdata/logic_test/builtin_function @@ -3036,7 +3036,7 @@ subtest crdb_internal.payloads_for_span # switch users -- this one has no permissions so expect errors user testuser -query error pq: only users with the admin role are allowed to use crdb_internal.payloads_for_span +query error user testuser does not have VIEWCLUSTERMETADATA system privilege SELECT * FROM crdb_internal.payloads_for_span(0) user root @@ -3052,7 +3052,7 @@ subtest crdb_internal.payloads_for_trace # switch users -- this one has no permissions so expect errors user testuser -query error pq: only users with the admin role are allowed to use crdb_internal.payloads_for_span +query error user testuser does not have VIEWCLUSTERMETADATA system privilege SELECT * FROM crdb_internal.payloads_for_span(0) user root diff --git a/pkg/sql/logictest/testdata/logic_test/crdb_internal b/pkg/sql/logictest/testdata/logic_test/crdb_internal index 6c36bfcaedea..f5e6da843e71 100644 --- a/pkg/sql/logictest/testdata/logic_test/crdb_internal +++ b/pkg/sql/logictest/testdata/logic_test/crdb_internal @@ -742,7 +742,7 @@ select crdb_internal.set_vmodule('') query error insufficient privilege select crdb_internal.get_vmodule() -query error pq: only users with the admin role are allowed to access the node runtime information +query error user testuser does not have VIEWCLUSTERMETADATA system privilege select * from crdb_internal.node_runtime_info query error pq: only users with the VIEWACTIVITY or VIEWACTIVITYREDACTED or ZONECONFIG privilege or the admin role can read crdb_internal.ranges_no_leases @@ -751,28 +751,28 @@ select * from crdb_internal.ranges query error pq: only users with the VIEWACTIVITY or VIEWACTIVITYREDACTED or ZONECONFIG privilege or the admin role can read crdb_internal.ranges_no_leases SHOW RANGES FROM TABLE foo -query error pq: only users with the admin role are allowed to read crdb_internal.gossip_nodes +query error user testuser does not have VIEWCLUSTERMETADATA system privilege select * from crdb_internal.gossip_nodes -query error pq: only users with the admin role are allowed to read crdb_internal.gossip_liveness +query error user testuser does not have VIEWCLUSTERMETADATA system privilege select * from crdb_internal.gossip_liveness -query error pq: only users with the admin role are allowed to read crdb_internal.node_metrics +query error user testuser does not have VIEWCLUSTERMETADATA system privilege select * from crdb_internal.node_metrics -query error pq: only users with the admin role are allowed to read crdb_internal.kv_node_status +query error user testuser does not have VIEWCLUSTERMETADATA system privilege select * from crdb_internal.kv_node_status -query error pq: only users with the admin role are allowed to read crdb_internal.kv_store_status +query error user testuser does not have VIEWCLUSTERMETADATA system privilege select * from crdb_internal.kv_store_status -query error pq: only users with the admin role are allowed to read crdb_internal.gossip_alerts +query error user testuser does not have VIEWCLUSTERMETADATA system privilege select * from crdb_internal.gossip_alerts -query error pq: only users with the admin role are allowed to read crdb_internal.node_inflight_trace_spans +query error user testuser does not have VIEWCLUSTERMETADATA system privilege select * from crdb_internal.node_inflight_trace_spans -query error pq: crdb_internal.check_consistency requires admin privileges +query error user testuser does not have REPAIRCLUSTERMETADATA system privilege SELECT * FROM crdb_internal.check_consistency(true, b'\x02', b'\x04') # Anyone can see the executable version. diff --git a/pkg/sql/logictest/testdata/logic_test/role b/pkg/sql/logictest/testdata/logic_test/role index 12cd99ef9de3..292c9d2d2a5b 100644 --- a/pkg/sql/logictest/testdata/logic_test/role +++ b/pkg/sql/logictest/testdata/logic_test/role @@ -1517,7 +1517,7 @@ user testuser # Verify that testuser is not allowed to edit some other role that has ADMIN. # The error message is important for this test -- we want to make sure it fails # because of a missing ADMIN power, not because of a missing CREATEROLE option. -statement error only users with the admin role are allowed to ALTER ROLE admin +statement error only users with the admin role are allowed to alter another admin ALTER ROLE other_admin NOCREATEDB subtest pw_hashes diff --git a/pkg/sql/logictest/testdata/logic_test/tenant_builtins b/pkg/sql/logictest/testdata/logic_test/tenant_builtins index 57f263ed8f7c..761a2d0ae1cb 100644 --- a/pkg/sql/logictest/testdata/logic_test/tenant_builtins +++ b/pkg/sql/logictest/testdata/logic_test/tenant_builtins @@ -272,7 +272,7 @@ SELECT crdb_internal.create_tenant('not-allowed') statement error user testuser does not have MANAGETENANT system privilege DROP TENANT [1] -statement error only users with the admin role are allowed to gc tenant +statement error crdb_internal.gc_tenant\(\): user testuser does not have REPAIRCLUSTERMETADATA system privilege SELECT crdb_internal.gc_tenant(314) user root diff --git a/pkg/sql/opt/cat/catalog.go b/pkg/sql/opt/cat/catalog.go index 377dafc7abba..47ad2f024ca8 100644 --- a/pkg/sql/opt/cat/catalog.go +++ b/pkg/sql/opt/cat/catalog.go @@ -177,10 +177,6 @@ type Catalog interface { // returns true. Returns an error if query on the `system.users` table failed HasAdminRole(ctx context.Context) (bool, error) - // RequireAdminRole checks that the current user has admin privileges. If not, - // returns an error. - RequireAdminRole(ctx context.Context, action string) error - // HasRoleOption converts the roleoption to its SQL column name and checks if // the user belongs to a role where the option has value true. Requires a // valid transaction to be open. diff --git a/pkg/sql/opt/optbuilder/BUILD.bazel b/pkg/sql/opt/optbuilder/BUILD.bazel index 4f2ec26e8db6..b80630334289 100644 --- a/pkg/sql/opt/optbuilder/BUILD.bazel +++ b/pkg/sql/opt/optbuilder/BUILD.bazel @@ -84,6 +84,7 @@ go_library( "//pkg/sql/sem/tree/treewindow", "//pkg/sql/sqlerrors", "//pkg/sql/sqltelemetry", + "//pkg/sql/syntheticprivilege", "//pkg/sql/types", "//pkg/util", "//pkg/util/errorutil", diff --git a/pkg/sql/opt/optbuilder/alter_range.go b/pkg/sql/opt/optbuilder/alter_range.go index 61cb87394716..a411ec637804 100644 --- a/pkg/sql/opt/optbuilder/alter_range.go +++ b/pkg/sql/opt/optbuilder/alter_range.go @@ -15,7 +15,9 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/opt" "github.com/cockroachdb/cockroach/pkg/sql/opt/memo" "github.com/cockroachdb/cockroach/pkg/sql/opt/props/physical" + "github.com/cockroachdb/cockroach/pkg/sql/privilege" "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" + "github.com/cockroachdb/cockroach/pkg/sql/syntheticprivilege" "github.com/cockroachdb/cockroach/pkg/sql/types" ) @@ -24,7 +26,7 @@ func (b *Builder) buildAlterRangeRelocate( relocate *tree.RelocateRange, inScope *scope, ) (outScope *scope) { - if err := b.catalog.RequireAdminRole(b.ctx, "ALTER RANGE RELOCATE"); err != nil { + if err := b.catalog.CheckPrivilege(b.ctx, syntheticprivilege.GlobalPrivilegeObject, privilege.REPAIRCLUSTERMETADATA); err != nil { panic(err) } diff --git a/pkg/sql/opt/testutils/testcat/BUILD.bazel b/pkg/sql/opt/testutils/testcat/BUILD.bazel index a6923903a5e7..8065ea682387 100644 --- a/pkg/sql/opt/testutils/testcat/BUILD.bazel +++ b/pkg/sql/opt/testutils/testcat/BUILD.bazel @@ -50,6 +50,7 @@ go_library( "//pkg/sql/sessiondatapb", "//pkg/sql/sqlerrors", "//pkg/sql/stats", + "//pkg/sql/syntheticprivilege", "//pkg/sql/types", "//pkg/sql/vtable", "//pkg/util/intsets", diff --git a/pkg/sql/opt/testutils/testcat/test_catalog.go b/pkg/sql/opt/testutils/testcat/test_catalog.go index b46f8c50d9db..aa7e8705900c 100644 --- a/pkg/sql/opt/testutils/testcat/test_catalog.go +++ b/pkg/sql/opt/testutils/testcat/test_catalog.go @@ -36,6 +36,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" "github.com/cockroachdb/cockroach/pkg/sql/sqlerrors" "github.com/cockroachdb/cockroach/pkg/sql/stats" + "github.com/cockroachdb/cockroach/pkg/sql/syntheticprivilege" "github.com/cockroachdb/cockroach/pkg/sql/types" "github.com/cockroachdb/cockroach/pkg/util/treeprinter" "github.com/cockroachdb/errors" @@ -283,6 +284,8 @@ func (tc *Catalog) CheckAnyPrivilege(ctx context.Context, o cat.Object) error { if t.Revoked { return pgerror.Newf(pgcode.InsufficientPrivilege, "user does not have privilege to access %v", t.SeqName) } + case *syntheticprivilege.GlobalPrivilege: + default: panic("invalid Object") } @@ -294,11 +297,6 @@ func (tc *Catalog) HasAdminRole(ctx context.Context) (bool, error) { return true, nil } -// RequireAdminRole is part of the cat.Catalog interface. -func (tc *Catalog) RequireAdminRole(ctx context.Context, action string) error { - return nil -} - // HasRoleOption is part of the cat.Catalog interface. func (tc *Catalog) HasRoleOption(ctx context.Context, roleOption roleoption.Option) (bool, error) { return true, nil diff --git a/pkg/sql/opt_catalog.go b/pkg/sql/opt_catalog.go index 74248f9e0aae..0d5268306f52 100644 --- a/pkg/sql/opt_catalog.go +++ b/pkg/sql/opt_catalog.go @@ -427,11 +427,6 @@ func (oc *optCatalog) HasAdminRole(ctx context.Context) (bool, error) { return oc.planner.HasAdminRole(ctx) } -// RequireAdminRole is part of the cat.Catalog interface. -func (oc *optCatalog) RequireAdminRole(ctx context.Context, action string) error { - return oc.planner.RequireAdminRole(ctx, action) -} - // HasRoleOption is part of the cat.Catalog interface. func (oc *optCatalog) HasRoleOption( ctx context.Context, roleOption roleoption.Option, diff --git a/pkg/sql/repair.go b/pkg/sql/repair.go index 3d650d2131e3..b37a68859773 100644 --- a/pkg/sql/repair.go +++ b/pkg/sql/repair.go @@ -37,6 +37,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror" "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgnotice" "github.com/cockroachdb/cockroach/pkg/sql/privilege" + "github.com/cockroachdb/cockroach/pkg/sql/syntheticprivilege" "github.com/cockroachdb/cockroach/pkg/storage" "github.com/cockroachdb/cockroach/pkg/util/duration" "github.com/cockroachdb/cockroach/pkg/util/hlc" @@ -778,7 +779,7 @@ func (p *planner) ForceDeleteTableData(ctx context.Context, descID int64) error } func (p *planner) ExternalReadFile(ctx context.Context, uri string) ([]byte, error) { - if err := p.RequireAdminRole(ctx, "network I/O"); err != nil { + if err := p.CheckPrivilege(ctx, syntheticprivilege.GlobalPrivilegeObject, privilege.REPAIRCLUSTERMETADATA); err != nil { return nil, err } @@ -795,7 +796,7 @@ func (p *planner) ExternalReadFile(ctx context.Context, uri string) ([]byte, err } func (p *planner) ExternalWriteFile(ctx context.Context, uri string, content []byte) error { - if err := p.RequireAdminRole(ctx, "network I/O"); err != nil { + if err := p.CheckPrivilege(ctx, syntheticprivilege.GlobalPrivilegeObject, privilege.REPAIRCLUSTERMETADATA); err != nil { return err } diff --git a/pkg/sql/scrub.go b/pkg/sql/scrub.go index f539c4175660..b7621cd150b2 100644 --- a/pkg/sql/scrub.go +++ b/pkg/sql/scrub.go @@ -18,8 +18,10 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/catalog" "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/sqlerrors" + "github.com/cockroachdb/cockroach/pkg/sql/syntheticprivilege" "github.com/cockroachdb/cockroach/pkg/util/hlc" "github.com/cockroachdb/errors" ) @@ -69,7 +71,7 @@ type checkOperation interface { // Scrub checks the database. // Privileges: superuser. func (p *planner) Scrub(ctx context.Context, n *tree.Scrub) (planNode, error) { - if err := p.RequireAdminRole(ctx, "SCRUB"); err != nil { + if err := p.CheckPrivilege(ctx, syntheticprivilege.GlobalPrivilegeObject, privilege.REPAIRCLUSTERMETADATA); err != nil { return nil, err } return &scrubNode{n: n}, nil diff --git a/pkg/sql/sem/builtins/BUILD.bazel b/pkg/sql/sem/builtins/BUILD.bazel index 47ced0b51589..467c9343cf08 100644 --- a/pkg/sql/sem/builtins/BUILD.bazel +++ b/pkg/sql/sem/builtins/BUILD.bazel @@ -96,6 +96,7 @@ go_library( "//pkg/sql/sqltelemetry", "//pkg/sql/storageparam", "//pkg/sql/storageparam/indexstorageparam", + "//pkg/sql/syntheticprivilege", "//pkg/sql/types", "//pkg/storage", "//pkg/util", diff --git a/pkg/sql/sem/builtins/generator_builtins.go b/pkg/sql/sem/builtins/generator_builtins.go index 89a47b3fceac..ef2270ca9a96 100644 --- a/pkg/sql/sem/builtins/generator_builtins.go +++ b/pkg/sql/sem/builtins/generator_builtins.go @@ -25,12 +25,14 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/lexbase" "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/protoreflect" "github.com/cockroachdb/cockroach/pkg/sql/sem/builtins/builtinconstants" "github.com/cockroachdb/cockroach/pkg/sql/sem/eval" "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" "github.com/cockroachdb/cockroach/pkg/sql/sem/volatility" "github.com/cockroachdb/cockroach/pkg/sql/sessiondata" + "github.com/cockroachdb/cockroach/pkg/sql/syntheticprivilege" "github.com/cockroachdb/cockroach/pkg/sql/types" "github.com/cockroachdb/cockroach/pkg/util/arith" "github.com/cockroachdb/cockroach/pkg/util/duration" @@ -1969,13 +1971,11 @@ var _ eval.ValueGenerator = &checkConsistencyGenerator{} func makeCheckConsistencyGenerator( ctx context.Context, evalCtx *eval.Context, args tree.Datums, ) (eval.ValueGenerator, error) { - isAdmin, err := evalCtx.SessionAccessor.HasAdminRole(ctx) - if err != nil { + if err := evalCtx.SessionAccessor.CheckPrivilege( + ctx, syntheticprivilege.GlobalPrivilegeObject, privilege.REPAIRCLUSTERMETADATA, + ); err != nil { return nil, err } - if !isAdmin { - return nil, pgerror.New(pgcode.InsufficientPrivilege, "crdb_internal.check_consistency requires admin privileges") - } keyFrom := roachpb.Key(*args[1].(*tree.DBytes)) keyTo := roachpb.Key(*args[2].(*tree.DBytes)) @@ -2340,17 +2340,11 @@ type payloadsForSpanGenerator struct { func makePayloadsForSpanGenerator( ctx context.Context, evalCtx *eval.Context, args tree.Datums, ) (eval.ValueGenerator, error) { - // The user must be an admin to use this builtin. - isAdmin, err := evalCtx.SessionAccessor.HasAdminRole(ctx) - if err != nil { + if err := evalCtx.SessionAccessor.CheckPrivilege( + ctx, syntheticprivilege.GlobalPrivilegeObject, privilege.VIEWCLUSTERMETADATA, + ); err != nil { return nil, err } - if !isAdmin { - return nil, pgerror.Newf( - pgcode.InsufficientPrivilege, - "only users with the admin role are allowed to use crdb_internal.payloads_for_span", - ) - } spanID := tracingpb.SpanID(*(args[0].(*tree.DInt))) span := evalCtx.Tracer.GetActiveSpanByID(spanID) if span == nil { @@ -2443,17 +2437,11 @@ type payloadsForTraceGenerator struct { func makePayloadsForTraceGenerator( ctx context.Context, evalCtx *eval.Context, args tree.Datums, ) (eval.ValueGenerator, error) { - // The user must be an admin to use this builtin. - isAdmin, err := evalCtx.SessionAccessor.HasAdminRole(ctx) - if err != nil { + if err := evalCtx.SessionAccessor.CheckPrivilege( + ctx, syntheticprivilege.GlobalPrivilegeObject, privilege.VIEWCLUSTERMETADATA, + ); err != nil { return nil, err } - if !isAdmin { - return nil, pgerror.Newf( - pgcode.InsufficientPrivilege, - "only users with the admin role are allowed to use crdb_internal.payloads_for_trace", - ) - } traceID := uint64(*(args[0].(*tree.DInt))) return &payloadsForTraceGenerator{traceID: traceID, planner: evalCtx.Planner}, nil } diff --git a/pkg/sql/sem/builtins/generator_probe_ranges.go b/pkg/sql/sem/builtins/generator_probe_ranges.go index cac95c4744c8..2bfa6a41b176 100644 --- a/pkg/sql/sem/builtins/generator_probe_ranges.go +++ b/pkg/sql/sem/builtins/generator_probe_ranges.go @@ -19,13 +19,13 @@ import ( "github.com/cockroachdb/cockroach/pkg/kv" "github.com/cockroachdb/cockroach/pkg/kv/kvclient" "github.com/cockroachdb/cockroach/pkg/roachpb" - "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/builtins/builtinconstants" "github.com/cockroachdb/cockroach/pkg/sql/sem/catid" "github.com/cockroachdb/cockroach/pkg/sql/sem/eval" "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" "github.com/cockroachdb/cockroach/pkg/sql/sem/volatility" + "github.com/cockroachdb/cockroach/pkg/sql/syntheticprivilege" "github.com/cockroachdb/cockroach/pkg/sql/types" "github.com/cockroachdb/cockroach/pkg/util/contextutil" "github.com/cockroachdb/cockroach/pkg/util/timeutil" @@ -125,17 +125,11 @@ type probeRangeGenerator struct { func makeProbeRangeGenerator( ctx context.Context, evalCtx *eval.Context, args tree.Datums, ) (eval.ValueGenerator, error) { - // The user must be an admin to use this builtin. - isAdmin, err := evalCtx.SessionAccessor.HasAdminRole(ctx) - if err != nil { + if err := evalCtx.SessionAccessor.CheckPrivilege( + ctx, syntheticprivilege.GlobalPrivilegeObject, privilege.REPAIRCLUSTERMETADATA, + ); err != nil { return nil, err } - if !isAdmin { - return nil, pgerror.Newf( - pgcode.InsufficientPrivilege, - "only users with the admin role are allowed to use crdb_internal.probe_range", - ) - } // Trace the query to meta2. Return it as part of the error string if the query fails. // This improves observability into a meta2 outage. We expect crdb_internal.probe_range // to be available, unless meta2 is down. @@ -147,6 +141,7 @@ func makeProbeRangeGenerator( ) defer sp.Finish() // Handle args passed in. + var err error ranges, err = kvclient.ScanMetaKVs(ctx, evalCtx.Txn, roachpb.Span{ Key: keys.MinKey, EndKey: keys.MaxKey, diff --git a/pkg/sql/set_zone_config.go b/pkg/sql/set_zone_config.go index 1e4f55db5703..50e689de4f6b 100644 --- a/pkg/sql/set_zone_config.go +++ b/pkg/sql/set_zone_config.go @@ -33,6 +33,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" "github.com/cockroachdb/cockroach/pkg/sql/sqlerrors" "github.com/cockroachdb/cockroach/pkg/sql/sqltelemetry" + "github.com/cockroachdb/cockroach/pkg/sql/syntheticprivilege" "github.com/cockroachdb/cockroach/pkg/sql/types" "github.com/cockroachdb/cockroach/pkg/util/errorutil" "github.com/cockroachdb/cockroach/pkg/util/log/eventpb" @@ -303,11 +304,11 @@ func checkPrivilegeForSetZoneConfig(ctx context.Context, p *planner, zs tree.Zon // an admin. Otherwise we require CREATE privileges on the database or table // in question. if zs.NamedZone != "" { - return p.RequireAdminRole(ctx, "alter system ranges") + return p.CheckPrivilege(ctx, syntheticprivilege.GlobalPrivilegeObject, privilege.REPAIRCLUSTERMETADATA) } if zs.Database != "" { if zs.Database == "system" { - return p.RequireAdminRole(ctx, "alter the system database") + return p.CheckPrivilege(ctx, syntheticprivilege.GlobalPrivilegeObject, privilege.REPAIRCLUSTERMETADATA) } dbDesc, err := p.Descriptors().ByNameWithLeased(p.txn).Get().Database(ctx, string(zs.Database)) if err != nil { @@ -333,7 +334,7 @@ func checkPrivilegeForSetZoneConfig(ctx context.Context, p *planner, zs tree.Zon return err } if tableDesc.GetParentID() == keys.SystemDatabaseID { - return p.RequireAdminRole(ctx, "alter system tables") + return p.CheckPrivilege(ctx, syntheticprivilege.GlobalPrivilegeObject, privilege.REPAIRCLUSTERMETADATA) } // Can set ZoneConfig if user has either CREATE privilege or ZONECONFIG privilege at the Table level diff --git a/pkg/sql/tenant_accessors.go b/pkg/sql/tenant_accessors.go index f00f4517eded..64c93d11795d 100644 --- a/pkg/sql/tenant_accessors.go +++ b/pkg/sql/tenant_accessors.go @@ -24,8 +24,10 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/isql" "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/sessiondata" + "github.com/cockroachdb/cockroach/pkg/sql/syntheticprivilege" "github.com/cockroachdb/cockroach/pkg/util/protoutil" "github.com/cockroachdb/errors" ) @@ -184,7 +186,7 @@ func (p *planner) LookupTenantID( ctx context.Context, tenantName roachpb.TenantName, ) (tid roachpb.TenantID, err error) { const op = "get-tenant-info" - if err := p.RequireAdminRole(ctx, op); err != nil { + if err := p.CheckPrivilege(ctx, syntheticprivilege.GlobalPrivilegeObject, privilege.VIEWCLUSTERMETADATA); err != nil { return tid, err } diff --git a/pkg/sql/tenant_gc.go b/pkg/sql/tenant_gc.go index df9d547326f5..9a4e9499711c 100644 --- a/pkg/sql/tenant_gc.go +++ b/pkg/sql/tenant_gc.go @@ -20,7 +20,9 @@ import ( "github.com/cockroachdb/cockroach/pkg/roachpb" "github.com/cockroachdb/cockroach/pkg/spanconfig" "github.com/cockroachdb/cockroach/pkg/sql/isql" + "github.com/cockroachdb/cockroach/pkg/sql/privilege" "github.com/cockroachdb/cockroach/pkg/sql/sessiondata" + "github.com/cockroachdb/cockroach/pkg/sql/syntheticprivilege" "github.com/cockroachdb/cockroach/pkg/util/hlc" "github.com/cockroachdb/cockroach/pkg/util/log" "github.com/cockroachdb/errors" @@ -133,7 +135,7 @@ func (p *planner) GCTenant(ctx context.Context, tenID uint64) error { if !p.extendedEvalCtx.TxnIsSingleStmt { return errors.Errorf("gc_tenant cannot be used inside a multi-statement transaction") } - if err := p.RequireAdminRole(ctx, "gc tenant"); err != nil { + if err := p.CheckPrivilege(ctx, syntheticprivilege.GlobalPrivilegeObject, privilege.REPAIRCLUSTERMETADATA); err != nil { return err } info, err := GetTenantRecordByID(ctx, p.InternalSQLTxn(), roachpb.MustMakeTenantID(tenID), p.ExecCfg().Settings) diff --git a/pkg/sql/tenant_settings.go b/pkg/sql/tenant_settings.go index 268c4f85d4e5..06b3d8ac93c6 100644 --- a/pkg/sql/tenant_settings.go +++ b/pkg/sql/tenant_settings.go @@ -19,9 +19,11 @@ import ( "github.com/cockroachdb/cockroach/pkg/settings/cluster" "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/eval" "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" "github.com/cockroachdb/cockroach/pkg/sql/sessiondata" + "github.com/cockroachdb/cockroach/pkg/sql/syntheticprivilege" "github.com/cockroachdb/cockroach/pkg/util/log/eventpb" "github.com/cockroachdb/errors" ) @@ -166,10 +168,7 @@ func (p *planner) ShowTenantClusterSetting( // privileged operation than viewing local cluster settings. So we // shouldn't be allowing with just the role option // VIEWCLUSTERSETTINGS. - // - // TODO(knz): Using admin authz for now; we may want to introduce a - // more specific role option later. - if err := p.RequireAdminRole(ctx, "view a tenant cluster setting"); err != nil { + if err := p.CheckPrivilege(ctx, syntheticprivilege.GlobalPrivilegeObject, privilege.VIEWCLUSTERMETADATA); err != nil { return nil, err } diff --git a/pkg/sql/tenant_update.go b/pkg/sql/tenant_update.go index 74f2475348a8..957109e18b36 100644 --- a/pkg/sql/tenant_update.go +++ b/pkg/sql/tenant_update.go @@ -23,8 +23,10 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/isql" "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/sessiondata" + "github.com/cockroachdb/cockroach/pkg/sql/syntheticprivilege" "github.com/cockroachdb/cockroach/pkg/util/log/logcrash" "github.com/cockroachdb/cockroach/pkg/util/protoutil" "github.com/cockroachdb/errors" @@ -151,7 +153,7 @@ func (p *planner) UpdateTenantResourceLimits( asOfConsumedRequestUnits float64, ) error { const op = "update-resource-limits" - if err := p.RequireAdminRole(ctx, "update tenant resource limits"); err != nil { + if err := p.CheckPrivilege(ctx, syntheticprivilege.GlobalPrivilegeObject, privilege.REPAIRCLUSTERMETADATA); err != nil { return err } diff --git a/pkg/sql/tests/copy_file_upload_test.go b/pkg/sql/tests/copy_file_upload_test.go index 0e3e8d417412..59eb85107677 100644 --- a/pkg/sql/tests/copy_file_upload_test.go +++ b/pkg/sql/tests/copy_file_upload_test.go @@ -295,7 +295,7 @@ func TestNodelocalNotAdmin(t *testing.T) { writeFile(t, testSendFile, fileContent) err = runCopyFile(t, userDB, smithUserName, testSendFile, sql.NodelocalFileUploadTable) - expectedErr := "only users with the admin role are allowed to upload" + expectedErr := "only users with the admin role are allowed to upload to nodelocal" require.True(t, testutils.IsError(err, expectedErr)) }