Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Added bool return value to RequireSuperUser function #38124

Closed
wants to merge 1 commit into from
Closed

Added bool return value to RequireSuperUser function #38124

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pkg/ccl/backupccl/backup.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/ccl/backupccl/restore.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/ccl/backupccl/show.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
8 changes: 7 additions & 1 deletion pkg/ccl/changefeedccl/changefeed_stmt.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/ccl/importccl/exportcsv.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/ccl/importccl/import_stmt.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/ccl/roleccl/role.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions pkg/server/status.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/alter_table.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
12 changes: 6 additions & 6 deletions pkg/sql/authorization.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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)
}

Expand Down
24 changes: 12 additions & 12 deletions pkg/sql/crdb_internal.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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 = ""
}
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/create_database.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/delegate/show_all_cluster_settings.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/opt/cat/catalog.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
4 changes: 2 additions & 2 deletions pkg/sql/opt/testutils/testcat/test_catalog.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/opt_catalog.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/rename_database.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/scrub.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/set_cluster_setting.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/show_cluster_setting.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down