diff --git a/pkg/ccl/backupccl/backup.go b/pkg/ccl/backupccl/backup.go index 433edcb09ea6..03ffe51ed2a5 100644 --- a/pkg/ccl/backupccl/backup.go +++ b/pkg/ccl/backupccl/backup.go @@ -840,7 +840,7 @@ func backupPlanHook( return err } - if err := p.RequireSuperUser(ctx, "BACKUP"); err != nil { + if _, err := p.RequireSuperUser(ctx, "BACKUP"); err != nil { return err } diff --git a/pkg/ccl/backupccl/restore.go b/pkg/ccl/backupccl/restore.go index 184183488759..2f67c8f8e434 100644 --- a/pkg/ccl/backupccl/restore.go +++ b/pkg/ccl/backupccl/restore.go @@ -1286,7 +1286,7 @@ func restorePlanHook( return err } - if err := p.RequireSuperUser(ctx, "RESTORE"); err != nil { + if _, err := p.RequireSuperUser(ctx, "RESTORE"); err != nil { return err } diff --git a/pkg/ccl/backupccl/show.go b/pkg/ccl/backupccl/show.go index f4970290c8d8..e1b9db212aac 100644 --- a/pkg/ccl/backupccl/show.go +++ b/pkg/ccl/backupccl/show.go @@ -38,7 +38,7 @@ func showBackupPlanHook( return nil, nil, nil, false, err } - if err := p.RequireSuperUser(ctx, "SHOW BACKUP"); err != nil { + if _, err := p.RequireSuperUser(ctx, "SHOW BACKUP"); err != nil { return nil, nil, nil, false, err } diff --git a/pkg/ccl/changefeedccl/changefeed_stmt.go b/pkg/ccl/changefeedccl/changefeed_stmt.go index 2ec642b49779..104901b3b01b 100644 --- a/pkg/ccl/changefeedccl/changefeed_stmt.go +++ b/pkg/ccl/changefeedccl/changefeed_stmt.go @@ -136,7 +136,13 @@ func changefeedPlanHook( ctx, span := tracing.ChildSpan(ctx, stmt.StatementTag()) defer tracing.FinishSpan(span) - if err := p.RequireSuperUser(ctx, "CREATE CHANGEFEED"); err != nil { + if !p.ExecCfg().Settings.Version.IsActive(cluster.VersionCreateChangefeed) { + return errors.Errorf(`CREATE CHANGEFEED requires all nodes to be upgraded to %s`, + cluster.VersionByKey(cluster.VersionCreateChangefeed), + ) + } + + if _, err := p.RequireSuperUser(ctx, "CREATE CHANGEFEED"); err != nil { return err } diff --git a/pkg/ccl/importccl/exportcsv.go b/pkg/ccl/importccl/exportcsv.go index d737a43acaa9..966388e50f0e 100644 --- a/pkg/ccl/importccl/exportcsv.go +++ b/pkg/ccl/importccl/exportcsv.go @@ -95,7 +95,7 @@ func exportPlanHook( return err } - if err := p.RequireSuperUser(ctx, "EXPORT"); err != nil { + if _, err := p.RequireSuperUser(ctx, "EXPORT"); err != nil { return err } diff --git a/pkg/ccl/importccl/import_stmt.go b/pkg/ccl/importccl/import_stmt.go index a4e135c2a564..594d1f33aeea 100644 --- a/pkg/ccl/importccl/import_stmt.go +++ b/pkg/ccl/importccl/import_stmt.go @@ -155,7 +155,7 @@ func importPlanHook( walltime := p.ExecCfg().Clock.Now().WallTime - if err := p.RequireSuperUser(ctx, "IMPORT"); err != nil { + if _, err := p.RequireSuperUser(ctx, "IMPORT"); err != nil { return err } diff --git a/pkg/ccl/roleccl/role.go b/pkg/ccl/roleccl/role.go index 9a0c6bbce8d4..afd2fbcd2b14 100644 --- a/pkg/ccl/roleccl/role.go +++ b/pkg/ccl/roleccl/role.go @@ -71,7 +71,7 @@ func grantRolePlanHook( return nil, err } - if err := p.RequireSuperUser(ctx, "grant role"); err != nil { + if _, err := p.RequireSuperUser(ctx, "grant role"); err != nil { // Not a superuser: check permissions on each role. allRoles, err := p.MemberOfWithAdminOption(ctx, p.User()) if err != nil { @@ -200,7 +200,7 @@ func revokeRolePlanHook( return nil, err } - if err := p.RequireSuperUser(ctx, "revoke role"); err != nil { + if _, err := p.RequireSuperUser(ctx, "revoke role"); err != nil { // Not a superuser: check permissions on each role. allRoles, err := p.MemberOfWithAdminOption(ctx, p.User()) if err != nil { diff --git a/pkg/server/status.go b/pkg/server/status.go index 32ad993e8180..e1da990a4a0c 100644 --- a/pkg/server/status.go +++ b/pkg/server/status.go @@ -1852,7 +1852,7 @@ func userFromContext(ctx context.Context) (string, error) { } type superUserChecker interface { - RequireSuperUser(ctx context.Context, action string) error + RequireSuperUser(ctx context.Context, action string) (bool, error) } func (s *statusServer) isSuperUser(ctx context.Context, username string) bool { @@ -1866,7 +1866,7 @@ func (s *statusServer) isSuperUser(ctx context.Context, username string) bool { &sql.MemoryMetrics{}, s.admin.server.execCfg) defer cleanup() - if err := planner.(superUserChecker).RequireSuperUser(ctx, "access status server endpoint"); err != nil { + if _, err := planner.(superUserChecker).RequireSuperUser(ctx, "access status server endpoint"); err != nil { return false } return true diff --git a/pkg/sql/alter_table.go b/pkg/sql/alter_table.go index e6a6d6660696..5e10208cc820 100644 --- a/pkg/sql/alter_table.go +++ b/pkg/sql/alter_table.go @@ -709,7 +709,7 @@ func (p *planner) setAuditMode( auditEvent{desc: desc, writing: true}) // We require root for now. Later maybe use a different permission? - if err := p.RequireSuperUser(ctx, "change auditing settings on a table"); err != nil { + if _, err := p.RequireSuperUser(ctx, "change auditing settings on a table"); err != nil { return false, err } diff --git a/pkg/sql/authorization.go b/pkg/sql/authorization.go index ed33d80a6a00..c501ade79209 100644 --- a/pkg/sql/authorization.go +++ b/pkg/sql/authorization.go @@ -47,7 +47,7 @@ type AuthorizationAccessor interface { // RequiresSuperUser errors if the session user isn't a super-user (i.e. root // or node). Includes the named action in the error message. - RequireSuperUser(ctx context.Context, action string) error + RequireSuperUser(ctx context.Context, action string) (bool, error) // MemberOfWithAdminOption looks up all the roles (direct and indirect) that 'member' is a member // of and returns a map of role -> isAdmin. @@ -145,26 +145,26 @@ func (p *planner) CheckAnyPrivilege(ctx context.Context, descriptor sqlbase.Desc } // RequireSuperUser implements the AuthorizationAccessor interface. -func (p *planner) RequireSuperUser(ctx context.Context, action string) error { +func (p *planner) RequireSuperUser(ctx context.Context, action string) (bool, error) { user := p.SessionData().User // Check if user is 'root' or 'node'. if user == security.RootUser || user == security.NodeUser { - return nil + return true, nil } // Expand role memberships. memberOf, err := p.MemberOfWithAdminOption(ctx, user) if err != nil { - return err + return false, err } // Check is 'user' is a member of role 'admin'. if _, ok := memberOf[sqlbase.AdminRole]; ok { - return nil + return true, nil } - return pgerror.Newf(pgerror.CodeInsufficientPrivilegeError, + return false, pgerror.Newf(pgerror.CodeInsufficientPrivilegeError, "only superusers are allowed to %s", action) } diff --git a/pkg/sql/crdb_internal.go b/pkg/sql/crdb_internal.go index 8a668ffe8849..cdbf120405f3 100644 --- a/pkg/sql/crdb_internal.go +++ b/pkg/sql/crdb_internal.go @@ -145,7 +145,7 @@ CREATE TABLE crdb_internal.node_runtime_info ( value STRING NOT NULL )`, populate: func(ctx context.Context, p *planner, _ *DatabaseDescriptor, addRow func(...tree.Datum) error) error { - if err := p.RequireSuperUser(ctx, "access the node runtime information"); err != nil { + if _, err := p.RequireSuperUser(ctx, "access the node runtime information"); err != nil { return err } @@ -593,7 +593,7 @@ CREATE TABLE crdb_internal.node_statement_statistics ( overhead_lat_var FLOAT NOT NULL )`, populate: func(ctx context.Context, p *planner, _ *DatabaseDescriptor, addRow func(...tree.Datum) error) error { - if err := p.RequireSuperUser(ctx, "access application statistics"); err != nil { + if _, err := p.RequireSuperUser(ctx, "access application statistics"); err != nil { return err } @@ -726,7 +726,7 @@ CREATE TABLE crdb_internal.cluster_settings ( description STRING NOT NULL )`, populate: func(ctx context.Context, p *planner, _ *DatabaseDescriptor, addRow func(...tree.Datum) error) error { - if err := p.RequireSuperUser(ctx, "read crdb_internal.cluster_settings"); err != nil { + if _, err := p.RequireSuperUser(ctx, "read crdb_internal.cluster_settings"); err != nil { return err } for _, k := range settings.Keys() { @@ -784,7 +784,7 @@ CREATE TABLE crdb_internal.%s ( func (p *planner) makeSessionsRequest(ctx context.Context) serverpb.ListSessionsRequest { req := serverpb.ListSessionsRequest{Username: p.SessionData().User} - if err := p.RequireSuperUser(ctx, "list sessions"); err == nil { + if _, err := p.RequireSuperUser(ctx, "list sessions"); err == nil { // The root user can see all sessions. req.Username = "" } @@ -1028,7 +1028,7 @@ var crdbInternalLocalMetricsTable = virtualSchemaTable{ value FLOAT NOT NULL -- value of the metric )`, populate: func(ctx context.Context, p *planner, _ *DatabaseDescriptor, addRow func(...tree.Datum) error) error { - if err := p.RequireSuperUser(ctx, "read crdb_internal.node_metrics"); err != nil { + if _, err := p.RequireSuperUser(ctx, "read crdb_internal.node_metrics"); err != nil { return err } @@ -1714,7 +1714,7 @@ CREATE TABLE crdb_internal.ranges_no_leases ( ) `, generator: func(ctx context.Context, p *planner, _ *DatabaseDescriptor) (virtualTableGenerator, error) { - if err := p.RequireSuperUser(ctx, "read crdb_internal.ranges_no_leases"); err != nil { + if _, err := p.RequireSuperUser(ctx, "read crdb_internal.ranges_no_leases"); err != nil { return nil, err } descs, err := p.Tables().getAllDescriptors(ctx, p.txn) @@ -1963,7 +1963,7 @@ CREATE TABLE crdb_internal.gossip_nodes ( ) `, populate: func(ctx context.Context, p *planner, _ *DatabaseDescriptor, addRow func(...tree.Datum) error) error { - if err := p.RequireSuperUser(ctx, "read crdb_internal.gossip_nodes"); err != nil { + if _, err := p.RequireSuperUser(ctx, "read crdb_internal.gossip_nodes"); err != nil { return err } @@ -2088,7 +2088,7 @@ CREATE TABLE crdb_internal.gossip_liveness ( // which is highly available. DO NOT CALL functions which require the // cluster to be healthy, such as StatusServer.Nodes(). - if err := p.RequireSuperUser(ctx, "read crdb_internal.gossip_liveness"); err != nil { + if _, err := p.RequireSuperUser(ctx, "read crdb_internal.gossip_liveness"); err != nil { return err } @@ -2156,7 +2156,7 @@ CREATE TABLE crdb_internal.gossip_alerts ( ) `, populate: func(ctx context.Context, p *planner, _ *DatabaseDescriptor, addRow func(...tree.Datum) error) error { - if err := p.RequireSuperUser(ctx, "read crdb_internal.gossip_alerts"); err != nil { + if _, err := p.RequireSuperUser(ctx, "read crdb_internal.gossip_alerts"); err != nil { return err } @@ -2222,7 +2222,7 @@ CREATE TABLE crdb_internal.gossip_network ( ) `, populate: func(ctx context.Context, p *planner, _ *DatabaseDescriptor, addRow func(...tree.Datum) error) error { - if err := p.RequireSuperUser(ctx, "read crdb_internal.gossip_network"); err != nil { + if _, err := p.RequireSuperUser(ctx, "read crdb_internal.gossip_network"); err != nil { return err } @@ -2341,7 +2341,7 @@ CREATE TABLE crdb_internal.kv_node_status ( ) `, populate: func(ctx context.Context, p *planner, _ *DatabaseDescriptor, addRow func(...tree.Datum) error) error { - if err := p.RequireSuperUser(ctx, "read crdb_internal.kv_node_status"); err != nil { + if _, err := p.RequireSuperUser(ctx, "read crdb_internal.kv_node_status"); err != nil { return err } @@ -2449,7 +2449,7 @@ CREATE TABLE crdb_internal.kv_store_status ( ) `, populate: func(ctx context.Context, p *planner, _ *DatabaseDescriptor, addRow func(...tree.Datum) error) error { - if err := p.RequireSuperUser(ctx, "read crdb_internal.kv_store_status"); err != nil { + if _, err := p.RequireSuperUser(ctx, "read crdb_internal.kv_store_status"); err != nil { return err } diff --git a/pkg/sql/create_database.go b/pkg/sql/create_database.go index b690d3d3d8ed..c3a5e6b3c2a5 100644 --- a/pkg/sql/create_database.go +++ b/pkg/sql/create_database.go @@ -67,7 +67,7 @@ func (p *planner) CreateDatabase(ctx context.Context, n *tree.CreateDatabase) (p } } - if err := p.RequireSuperUser(ctx, "CREATE DATABASE"); err != nil { + if _, err := p.RequireSuperUser(ctx, "CREATE DATABASE"); err != nil { return nil, err } diff --git a/pkg/sql/delegate/show_all_cluster_settings.go b/pkg/sql/delegate/show_all_cluster_settings.go index 56c16c726ca9..e4b220019ae7 100644 --- a/pkg/sql/delegate/show_all_cluster_settings.go +++ b/pkg/sql/delegate/show_all_cluster_settings.go @@ -17,7 +17,7 @@ import "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" func (d *delegator) delegateShowAllClusterSettings( stmt *tree.ShowAllClusterSettings, ) (tree.Statement, error) { - if err := d.catalog.RequireSuperUser(d.ctx, "SHOW ALL CLUSTER SETTINGS"); err != nil { + if _, err := d.catalog.RequireSuperUser(d.ctx, "SHOW ALL CLUSTER SETTINGS"); err != nil { return nil, err } return parse( diff --git a/pkg/sql/opt/cat/catalog.go b/pkg/sql/opt/cat/catalog.go index ce668d6b0aa5..d5c4b28b76d7 100644 --- a/pkg/sql/opt/cat/catalog.go +++ b/pkg/sql/opt/cat/catalog.go @@ -121,5 +121,5 @@ type Catalog interface { // RequireSuperUser checks that the current user has admin privileges. If not, // returns an error. - RequireSuperUser(ctx context.Context, action string) error + RequireSuperUser(ctx context.Context, action string) (bool, error) } diff --git a/pkg/sql/opt/testutils/testcat/test_catalog.go b/pkg/sql/opt/testutils/testcat/test_catalog.go index 8ba9f877c414..36187eb06b38 100644 --- a/pkg/sql/opt/testutils/testcat/test_catalog.go +++ b/pkg/sql/opt/testutils/testcat/test_catalog.go @@ -192,8 +192,8 @@ func (tc *Catalog) CheckAnyPrivilege(ctx context.Context, o cat.Object) error { } // RequireSuperUser is part of the cat.Catalog interface. -func (tc *Catalog) RequireSuperUser(ctx context.Context, action string) error { - return nil +func (tc *Catalog) RequireSuperUser(ctx context.Context, action string) (bool, error) { + return true, nil } func (tc *Catalog) resolveSchema(toResolve *cat.SchemaName) (cat.Schema, cat.SchemaName, error) { diff --git a/pkg/sql/opt_catalog.go b/pkg/sql/opt_catalog.go index e1eb327cc7b2..5b6266213512 100644 --- a/pkg/sql/opt_catalog.go +++ b/pkg/sql/opt_catalog.go @@ -233,7 +233,7 @@ func (oc *optCatalog) CheckAnyPrivilege(ctx context.Context, o cat.Object) error } // RequireSuperUser is part of the cat.Catalog interface. -func (oc *optCatalog) RequireSuperUser(ctx context.Context, action string) error { +func (oc *optCatalog) RequireSuperUser(ctx context.Context, action string) (bool, error) { return oc.planner.RequireSuperUser(ctx, action) } diff --git a/pkg/sql/rename_database.go b/pkg/sql/rename_database.go index b30da8ca1b5f..0d481a2bdda7 100644 --- a/pkg/sql/rename_database.go +++ b/pkg/sql/rename_database.go @@ -41,7 +41,7 @@ func (p *planner) RenameDatabase(ctx context.Context, n *tree.RenameDatabase) (p return nil, pgerror.DangerousStatementf("RENAME DATABASE on current database") } - if err := p.RequireSuperUser(ctx, "ALTER DATABASE ... RENAME"); err != nil { + if _, err := p.RequireSuperUser(ctx, "ALTER DATABASE ... RENAME"); err != nil { return nil, err } diff --git a/pkg/sql/scrub.go b/pkg/sql/scrub.go index 32495660a940..747c3e102680 100644 --- a/pkg/sql/scrub.go +++ b/pkg/sql/scrub.go @@ -73,7 +73,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.RequireSuperUser(ctx, "SCRUB"); err != nil { + if _, err := p.RequireSuperUser(ctx, "SCRUB"); err != nil { return nil, err } return &scrubNode{n: n}, nil diff --git a/pkg/sql/set_cluster_setting.go b/pkg/sql/set_cluster_setting.go index c393995eed56..20f42757fdde 100644 --- a/pkg/sql/set_cluster_setting.go +++ b/pkg/sql/set_cluster_setting.go @@ -46,7 +46,7 @@ type setClusterSettingNode struct { func (p *planner) SetClusterSetting( ctx context.Context, n *tree.SetClusterSetting, ) (planNode, error) { - if err := p.RequireSuperUser(ctx, "SET CLUSTER SETTING"); err != nil { + if _, err := p.RequireSuperUser(ctx, "SET CLUSTER SETTING"); err != nil { return nil, err } diff --git a/pkg/sql/show_cluster_setting.go b/pkg/sql/show_cluster_setting.go index 0219bbcbc467..da3263fb14a0 100644 --- a/pkg/sql/show_cluster_setting.go +++ b/pkg/sql/show_cluster_setting.go @@ -106,7 +106,7 @@ func (p *planner) ShowClusterSetting( ctx context.Context, n *tree.ShowClusterSetting, ) (planNode, error) { - if err := p.RequireSuperUser(ctx, "SHOW CLUSTER SETTING"); err != nil { + if _, err := p.RequireSuperUser(ctx, "SHOW CLUSTER SETTING"); err != nil { return nil, err }