From a83f88c9855a8610eea4eb2089e4aa5528696fec Mon Sep 17 00:00:00 2001 From: Andy Yang Date: Wed, 6 Sep 2023 00:34:42 -0400 Subject: [PATCH 1/4] sql: add CREATELOGIN system privilege Release note (sql change): There is now a `CREATELOGIN` system privilege, which is analogous to the existing `CREATELOGIN` role option, but can also be inherited by role membership. --- pkg/sql/alter_role.go | 2 +- pkg/sql/logictest/testdata/logic_test/role | 67 ++++++++++++++++++++++ pkg/sql/privilege/kind_string.go | 5 +- pkg/sql/privilege/privilege.go | 5 +- 4 files changed, 75 insertions(+), 4 deletions(-) diff --git a/pkg/sql/alter_role.go b/pkg/sql/alter_role.go index 9a8445082326..6989ce5dd849 100644 --- a/pkg/sql/alter_role.go +++ b/pkg/sql/alter_role.go @@ -135,7 +135,7 @@ func (p *planner) checkPasswordOptionConstraints( // NOCREATELOGIN to another role, or set up a password for // authentication, or set up password validity, or enable/disable // LOGIN privilege; even if they have CREATEROLE privilege. - if err := p.CheckRoleOption(ctx, roleoption.CREATELOGIN); err != nil { + if err := p.CheckGlobalPrivilegeOrRoleOption(ctx, privilege.CREATELOGIN); err != nil { return err } } diff --git a/pkg/sql/logictest/testdata/logic_test/role b/pkg/sql/logictest/testdata/logic_test/role index b7f061231f79..5cc31a13a1b0 100644 --- a/pkg/sql/logictest/testdata/logic_test/role +++ b/pkg/sql/logictest/testdata/logic_test/role @@ -1725,3 +1725,70 @@ statement ok DROP ROLE parent_with_createrole subtest end + +subtest create_login_priv + +user root + +statement ok +ALTER ROLE testuser CREATEROLE NOCREATELOGIN + +user testuser + +statement error user testuser does not have CREATELOGIN privilege +CREATE USER u_create_login_priv WITH PASSWORD 'roacher4lyfe' + +user root + +statement ok +GRANT SYSTEM CREATELOGIN TO testuser + +user testuser + +statement ok +CREATE USER u_create_login_priv WITH PASSWORD 'roacher4lyfe' + +statement ok +DROP USER u_create_login_priv + +user root + +statement ok +REVOKE SYSTEM CREATELOGIN FROM testuser + +user testuser + +statement error user testuser does not have CREATELOGIN privilege +CREATE USER u_create_login_priv WITH PASSWORD 'roacher4lyfe' + +user root + +statement ok +CREATE ROLE creator_of_logins + +statement ok +GRANT SYSTEM CREATELOGIN TO creator_of_logins + +statement ok +GRANT creator_of_logins TO testuser + +user testuser + +statement ok +CREATE USER u_create_login_priv WITH PASSWORD 'roacher4lyfe' + +statement ok +DROP USER u_create_login_priv + +user root + +statement ok +REVOKE SYSTEM CREATELOGIN FROM creator_of_logins + +statement ok +DROP ROLE creator_of_logins + +statement ok +ALTER ROLE testuser NOCREATEROLE + +subtest end diff --git a/pkg/sql/privilege/kind_string.go b/pkg/sql/privilege/kind_string.go index 7257b3710c36..a0c6646c17ee 100644 --- a/pkg/sql/privilege/kind_string.go +++ b/pkg/sql/privilege/kind_string.go @@ -40,7 +40,8 @@ func _() { _ = x[MANAGETENANT-30] _ = x[VIEWSYSTEMTABLE-31] _ = x[CREATEROLE-32] - _ = x[largestKind-32] + _ = x[CREATELOGIN-33] + _ = x[largestKind-33] } func (i Kind) String() string { @@ -109,6 +110,8 @@ func (i Kind) String() string { return "VIEWSYSTEMTABLE" case CREATEROLE: return "CREATEROLE" + case CREATELOGIN: + return "CREATELOGIN" default: return "Kind(" + strconv.FormatInt(int64(i), 10) + ")" } diff --git a/pkg/sql/privilege/privilege.go b/pkg/sql/privilege/privilege.go index 06b7245c042e..daf0318162f5 100644 --- a/pkg/sql/privilege/privilege.go +++ b/pkg/sql/privilege/privilege.go @@ -73,7 +73,8 @@ const ( MANAGETENANT Kind = 30 VIEWSYSTEMTABLE Kind = 31 CREATEROLE Kind = 32 - largestKind = CREATEROLE + CREATELOGIN Kind = 33 + largestKind = CREATELOGIN ) var isDeprecatedKind = map[Kind]bool{ @@ -161,7 +162,7 @@ var ( GlobalPrivileges = List{ ALL, BACKUP, RESTORE, MODIFYCLUSTERSETTING, EXTERNALCONNECTION, VIEWACTIVITY, VIEWACTIVITYREDACTED, VIEWCLUSTERSETTING, CANCELQUERY, NOSQLLOGIN, VIEWCLUSTERMETADATA, VIEWDEBUG, EXTERNALIOIMPLICITACCESS, VIEWJOB, - MODIFYSQLCLUSTERSETTING, REPLICATION, MANAGETENANT, VIEWSYSTEMTABLE, CREATEROLE, + MODIFYSQLCLUSTERSETTING, REPLICATION, MANAGETENANT, VIEWSYSTEMTABLE, CREATEROLE, CREATELOGIN, } VirtualTablePrivileges = List{ALL, SELECT} ExternalConnectionPrivileges = List{ALL, USAGE, DROP} From 0e248a4785ff03c4ce72df38cb1b80ea4408b148 Mon Sep 17 00:00:00 2001 From: Andy Yang Date: Wed, 6 Sep 2023 02:18:30 -0400 Subject: [PATCH 2/4] sql: add CREATEDB system privilege Release note (sql change): There is now a `CREATEDB` system privilege, which is analogous to the existing `CREATEDB` role option, but can also be inherited by role membership. --- pkg/ccl/backupccl/BUILD.bazel | 1 - pkg/ccl/backupccl/restore_planning.go | 3 +- pkg/sql/alter_database.go | 3 +- pkg/sql/create_database.go | 4 +- pkg/sql/logictest/testdata/logic_test/role | 61 ++++++++++++++++++++++ pkg/sql/privilege/kind_string.go | 5 +- pkg/sql/privilege/privilege.go | 5 +- pkg/sql/rename_database.go | 3 +- 8 files changed, 73 insertions(+), 12 deletions(-) diff --git a/pkg/ccl/backupccl/BUILD.bazel b/pkg/ccl/backupccl/BUILD.bazel index bea71847b539..b71456317b0d 100644 --- a/pkg/ccl/backupccl/BUILD.bazel +++ b/pkg/ccl/backupccl/BUILD.bazel @@ -109,7 +109,6 @@ go_library( "//pkg/sql/physicalplan", "//pkg/sql/privilege", "//pkg/sql/protoreflect", - "//pkg/sql/roleoption", "//pkg/sql/rowenc", "//pkg/sql/rowexec", "//pkg/sql/schemachanger/scbackup", diff --git a/pkg/ccl/backupccl/restore_planning.go b/pkg/ccl/backupccl/restore_planning.go index 94e3a4b3b3a4..cdac6293d810 100644 --- a/pkg/ccl/backupccl/restore_planning.go +++ b/pkg/ccl/backupccl/restore_planning.go @@ -56,7 +56,6 @@ 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/roleoption" "github.com/cockroachdb/cockroach/pkg/sql/sem/catconstants" "github.com/cockroachdb/cockroach/pkg/sql/sem/catid" "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" @@ -1556,7 +1555,7 @@ func checkPrivilegesForRestore( "RESTORE system privilege.", deprecatedPrivilegesRestorePreamble, p.User().Normalized()) p.BufferClientNotice(ctx, pgnotice.Newf("%s", notice)) - hasCreateDB, err := p.HasRoleOption(ctx, roleoption.CREATEDB) + hasCreateDB, err := p.HasGlobalPrivilegeOrRoleOption(ctx, privilege.CREATEDB) if err != nil { return err } diff --git a/pkg/sql/alter_database.go b/pkg/sql/alter_database.go index d54a0c8a4596..603f48154aa4 100644 --- a/pkg/sql/alter_database.go +++ b/pkg/sql/alter_database.go @@ -33,7 +33,6 @@ 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/roleoption" "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" "github.com/cockroachdb/cockroach/pkg/sql/sqlerrors" "github.com/cockroachdb/cockroach/pkg/sql/sqltelemetry" @@ -84,7 +83,7 @@ func (n *alterDatabaseOwnerNode) startExec(params runParams) error { } // To alter the owner, the user also has to have CREATEDB privilege. - if err := params.p.CheckRoleOption(params.ctx, roleoption.CREATEDB); err != nil { + if err := params.p.CheckGlobalPrivilegeOrRoleOption(params.ctx, privilege.CREATEDB); err != nil { return err } diff --git a/pkg/sql/create_database.go b/pkg/sql/create_database.go index 7e06c9324207..9bb231896e15 100644 --- a/pkg/sql/create_database.go +++ b/pkg/sql/create_database.go @@ -17,7 +17,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/server/telemetry" "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode" "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror" - "github.com/cockroachdb/cockroach/pkg/sql/roleoption" + "github.com/cockroachdb/cockroach/pkg/sql/privilege" "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" "github.com/cockroachdb/cockroach/pkg/sql/sqltelemetry" "github.com/cockroachdb/cockroach/pkg/util/errorutil/unimplemented" @@ -148,7 +148,7 @@ func (p *planner) CreateDatabase(ctx context.Context, n *tree.CreateDatabase) (p // CanCreateDatabase verifies that the current user has the CREATEDB // role option. func (p *planner) CanCreateDatabase(ctx context.Context) error { - hasCreateDB, err := p.HasRoleOption(ctx, roleoption.CREATEDB) + hasCreateDB, err := p.HasGlobalPrivilegeOrRoleOption(ctx, privilege.CREATEDB) if err != nil { return err } diff --git a/pkg/sql/logictest/testdata/logic_test/role b/pkg/sql/logictest/testdata/logic_test/role index 5cc31a13a1b0..edf51c22d265 100644 --- a/pkg/sql/logictest/testdata/logic_test/role +++ b/pkg/sql/logictest/testdata/logic_test/role @@ -1792,3 +1792,64 @@ statement ok ALTER ROLE testuser NOCREATEROLE subtest end + +subtest create_db_priv + +user root + +statement ok +ALTER ROLE testuser NOCREATEDB + +user testuser + +statement error permission denied to create database +CREATE DATABASE db_create_db_priv + +user root + +statement ok +GRANT SYSTEM CREATEDB TO testuser + +user testuser + +statement ok +CREATE DATABASE db_create_db_priv + +user root + +statement ok +REVOKE SYSTEM CREATEDB FROM testuser + +user testuser + +statement error permission denied to rename database +ALTER DATABASE db_create_db_priv RENAME TO renamed_db_create_db_priv + +user root + +statement ok +CREATE ROLE creator_of_databases + +statement ok +GRANT creator_of_databases TO testuser + +statement ok +GRANT SYSTEM CREATEDB TO creator_of_databases + +user testuser + +statement ok +ALTER DATABASE db_create_db_priv RENAME TO renamed_db_create_db_priv + +user root + +statement ok +DROP DATABASE renamed_db_create_db_priv + +statement ok +REVOKE SYSTEM CREATEDB FROM creator_of_databases + +statement ok +DROP ROLE creator_of_databases + +subtest end diff --git a/pkg/sql/privilege/kind_string.go b/pkg/sql/privilege/kind_string.go index a0c6646c17ee..ff35bd36bc55 100644 --- a/pkg/sql/privilege/kind_string.go +++ b/pkg/sql/privilege/kind_string.go @@ -41,7 +41,8 @@ func _() { _ = x[VIEWSYSTEMTABLE-31] _ = x[CREATEROLE-32] _ = x[CREATELOGIN-33] - _ = x[largestKind-33] + _ = x[CREATEDB-34] + _ = x[largestKind-34] } func (i Kind) String() string { @@ -112,6 +113,8 @@ func (i Kind) String() string { return "CREATEROLE" case CREATELOGIN: return "CREATELOGIN" + case CREATEDB: + return "CREATEDB" default: return "Kind(" + strconv.FormatInt(int64(i), 10) + ")" } diff --git a/pkg/sql/privilege/privilege.go b/pkg/sql/privilege/privilege.go index daf0318162f5..2fcdafd8dc09 100644 --- a/pkg/sql/privilege/privilege.go +++ b/pkg/sql/privilege/privilege.go @@ -74,7 +74,8 @@ const ( VIEWSYSTEMTABLE Kind = 31 CREATEROLE Kind = 32 CREATELOGIN Kind = 33 - largestKind = CREATELOGIN + CREATEDB Kind = 34 + largestKind = CREATEDB ) var isDeprecatedKind = map[Kind]bool{ @@ -162,7 +163,7 @@ var ( GlobalPrivileges = List{ ALL, BACKUP, RESTORE, MODIFYCLUSTERSETTING, EXTERNALCONNECTION, VIEWACTIVITY, VIEWACTIVITYREDACTED, VIEWCLUSTERSETTING, CANCELQUERY, NOSQLLOGIN, VIEWCLUSTERMETADATA, VIEWDEBUG, EXTERNALIOIMPLICITACCESS, VIEWJOB, - MODIFYSQLCLUSTERSETTING, REPLICATION, MANAGETENANT, VIEWSYSTEMTABLE, CREATEROLE, CREATELOGIN, + MODIFYSQLCLUSTERSETTING, REPLICATION, MANAGETENANT, VIEWSYSTEMTABLE, CREATEROLE, CREATELOGIN, CREATEDB, } VirtualTablePrivileges = List{ALL, SELECT} ExternalConnectionPrivileges = List{ALL, USAGE, DROP} diff --git a/pkg/sql/rename_database.go b/pkg/sql/rename_database.go index 8a3ea2051256..65d4e14a2e37 100644 --- a/pkg/sql/rename_database.go +++ b/pkg/sql/rename_database.go @@ -20,7 +20,6 @@ import ( "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/roleoption" "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" "github.com/cockroachdb/cockroach/pkg/sql/sqlerrors" "github.com/cockroachdb/cockroach/pkg/util/log" @@ -81,7 +80,7 @@ func (p *planner) RenameDatabase(ctx context.Context, n *tree.RenameDatabase) (p return nil, pgerror.Newf( pgcode.InsufficientPrivilege, "must be owner of database %s", n.Name) } - hasCreateDB, err := p.HasRoleOption(ctx, roleoption.CREATEDB) + hasCreateDB, err := p.HasGlobalPrivilegeOrRoleOption(ctx, privilege.CREATEDB) if err != nil { return nil, err } From 17aec43bb64df6c1ee41f21d82f8c79c21d2f5b2 Mon Sep 17 00:00:00 2001 From: Andy Yang Date: Thu, 7 Sep 2023 15:27:05 -0400 Subject: [PATCH 3/4] jobsauth: short circuit privilege check for ViewAccess This patch updates the job authorization code to only check the VIEWJOB privilege if the user does not have CONTROLJOB for ViewAccess. It also updates the comment to reflect granting access when a user has VIEWJOB and corrects a format specifier. Release note: None --- pkg/jobs/jobsauth/authorization.go | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/pkg/jobs/jobsauth/authorization.go b/pkg/jobs/jobsauth/authorization.go index 2708ca2c1bd1..24b2bef69475 100644 --- a/pkg/jobs/jobsauth/authorization.go +++ b/pkg/jobs/jobsauth/authorization.go @@ -81,7 +81,7 @@ type AuthorizationAccessor interface { // TODO(#96432): sort out internal job owners and rules for accessing them // Authorize checks these rules in order: // 1. If the user is an admin, grant access. -// 2. If the AccessLevel is ViewAccess, grant access if the user has CONTROLJOB +// 2. If the AccessLevel is ViewAccess, grant access if the user has CONTROLJOB, VIEWJOB, // or if the user owns the job. // 3. If the AccessLevel is ControlAccess, grant access if the user has CONTROLJOB // and the job owner is not an admin. @@ -106,15 +106,17 @@ func Authorize( return err } - hasViewJob, err := a.HasPrivilege(ctx, &syntheticprivilege.GlobalPrivilege{}, privilege.VIEWJOB, a.User()) - if err != nil { - return err - } - jobOwnerUser := payload.UsernameProto.Decode() if accessLevel == ViewAccess { - if a.User() == jobOwnerUser || hasControlJob || hasViewJob { + if a.User() == jobOwnerUser || hasControlJob { + return nil + } + hasViewJob, err := a.HasPrivilege(ctx, &syntheticprivilege.GlobalPrivilege{}, privilege.VIEWJOB, a.User()) + if err != nil { + return err + } + if hasViewJob { return nil } } @@ -136,6 +138,6 @@ func Authorize( return check(ctx, a, jobID, payload) } return pgerror.Newf(pgcode.InsufficientPrivilege, - "user %s does not have privileges for job $d", + "user %s does not have privileges for job %d", a.User(), jobID) } From 81eb17e46eec9ccd7d45db950095506aa0b97233 Mon Sep 17 00:00:00 2001 From: Andy Yang Date: Thu, 7 Sep 2023 11:55:07 -0400 Subject: [PATCH 4/4] sql: add CONTROLJOB system privilege Release note (sql change): There is now a `CONTROLJOB` system privilege, which is analogous to the existing `CONTROLJOB` role option, but can also be inherited by role membership. --- pkg/jobs/jobsauth/BUILD.bazel | 1 + pkg/jobs/jobsauth/authorization.go | 5 +- pkg/jobs/jobsauth/authorization_test.go | 44 ++++++++++++- pkg/sql/logictest/testdata/logic_test/jobs | 74 ++++++++++++++++++++++ pkg/sql/privilege/kind_string.go | 5 +- pkg/sql/privilege/privilege.go | 5 +- 6 files changed, 128 insertions(+), 6 deletions(-) diff --git a/pkg/jobs/jobsauth/BUILD.bazel b/pkg/jobs/jobsauth/BUILD.bazel index ff090573092a..8545fefb9349 100644 --- a/pkg/jobs/jobsauth/BUILD.bazel +++ b/pkg/jobs/jobsauth/BUILD.bazel @@ -32,6 +32,7 @@ go_test( "//pkg/sql/pgwire/pgerror", "//pkg/sql/privilege", "//pkg/sql/roleoption", + "//pkg/sql/syntheticprivilege", "//pkg/util/randutil", "@com_github_stretchr_testify//assert", ], diff --git a/pkg/jobs/jobsauth/authorization.go b/pkg/jobs/jobsauth/authorization.go index 24b2bef69475..2595dc3ae805 100644 --- a/pkg/jobs/jobsauth/authorization.go +++ b/pkg/jobs/jobsauth/authorization.go @@ -70,6 +70,9 @@ type AuthorizationAccessor interface { // HasAdminRole mirrors sql.AuthorizationAccessor. HasAdminRole(ctx context.Context) (bool, error) + // HasGlobalPrivilegeOrRoleOption mirrors sql.AuthorizationAccessor. + HasGlobalPrivilegeOrRoleOption(ctx context.Context, privilege privilege.Kind) (bool, error) + // User mirrors sql.PlanHookState. User() username.SQLUsername } @@ -101,7 +104,7 @@ func Authorize( return nil } - hasControlJob, err := a.HasRoleOption(ctx, roleoption.CONTROLJOB) + hasControlJob, err := a.HasGlobalPrivilegeOrRoleOption(ctx, privilege.CONTROLJOB) if err != nil { return err } diff --git a/pkg/jobs/jobsauth/authorization_test.go b/pkg/jobs/jobsauth/authorization_test.go index 5ff7f5f6b990..7678e299cbfb 100644 --- a/pkg/jobs/jobsauth/authorization_test.go +++ b/pkg/jobs/jobsauth/authorization_test.go @@ -26,6 +26,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror" "github.com/cockroachdb/cockroach/pkg/sql/privilege" "github.com/cockroachdb/cockroach/pkg/sql/roleoption" + "github.com/cockroachdb/cockroach/pkg/sql/syntheticprivilege" "github.com/cockroachdb/cockroach/pkg/util/randutil" "github.com/stretchr/testify/assert" ) @@ -37,6 +38,8 @@ type userPrivilege struct { var viewJobGlobalPrivilege = userPrivilege{privilege.VIEWJOB, privilege.Global} +var controlJobGlobalPrivilege = userPrivilege{privilege.CONTROLJOB, privilege.Global} + type testAuthAccessor struct { user username.SQLUsername @@ -112,6 +115,22 @@ func (a *testAuthAccessor) HasAdminRole(ctx context.Context) (bool, error) { return ok, nil } +func (a *testAuthAccessor) HasGlobalPrivilegeOrRoleOption( + ctx context.Context, privilege privilege.Kind, +) (bool, error) { + ok, err := a.HasPrivilege(ctx, syntheticprivilege.GlobalPrivilegeObject, privilege, a.User()) + if err != nil { + return false, err + } + if ok { + return true, nil + } + if roleOption, ok := roleoption.ByName[privilege.String()]; ok { + return a.HasRoleOption(ctx, roleOption) + } + return false, nil +} + func (a *testAuthAccessor) User() username.SQLUsername { return a.user } @@ -157,7 +176,7 @@ func TestAuthorization(t *testing.T) { userErr error }{ { - name: "controljob-sufficient-for-non-admin-jobs", + name: "controljob-role-option-sufficient-for-non-admin-jobs", user: username.MakeSQLUsernameFromPreNormalizedString("user1"), roleOptions: map[roleoption.Option]struct{}{ @@ -168,7 +187,7 @@ func TestAuthorization(t *testing.T) { accessLevel: jobsauth.ControlAccess, }, { - name: "controljob-sufficient-to-view-admin-jobs", + name: "controljob-role-option-sufficient-to-view-admin-jobs", user: username.MakeSQLUsernameFromPreNormalizedString("user1"), roleOptions: map[roleoption.Option]struct{}{ roleoption.CONTROLJOB: {}, @@ -262,6 +281,27 @@ func TestAuthorization(t *testing.T) { accessLevel: jobsauth.ControlAccess, userErr: pgerror.New(pgcode.InsufficientPrivilege, "foo"), }, + { + name: "controljob-system-privilege-sufficient-for-non-admin-jobs", + + user: username.MakeSQLUsernameFromPreNormalizedString("user1"), + userPrivileges: map[userPrivilege]struct{}{ + controlJobGlobalPrivilege: {}, + }, + admins: map[string]struct{}{}, + payload: makeBackupPayload("user2"), + accessLevel: jobsauth.ControlAccess, + }, + { + name: "controljob-system-privilege-sufficient-to-view-admin-jobs", + user: username.MakeSQLUsernameFromPreNormalizedString("user1"), + userPrivileges: map[userPrivilege]struct{}{ + controlJobGlobalPrivilege: {}, + }, + admins: map[string]struct{}{"user2": {}}, + payload: makeBackupPayload("user2"), + accessLevel: jobsauth.ViewAccess, + }, } { t.Run(tc.name, func(t *testing.T) { testAuth := &testAuthAccessor{ diff --git a/pkg/sql/logictest/testdata/logic_test/jobs b/pkg/sql/logictest/testdata/logic_test/jobs index d9a2a1a4def4..52c2bf4895a5 100644 --- a/pkg/sql/logictest/testdata/logic_test/jobs +++ b/pkg/sql/logictest/testdata/logic_test/jobs @@ -280,3 +280,77 @@ query I SELECT count(*) FROM [SHOW AUTOMATIC JOBS] WHERE job_type = 'AUTO CONFIG RUNNER' AND status = 'running' ---- 1 + +subtest control_job_priv + +user testuser + +statement ok +CREATE TABLE t_control_job_priv(x INT); +DROP TABLE t_control_job_priv + +let $job_id +SELECT job_id FROM [SHOW JOBS] WHERE user_name = 'testuser' AND job_type = 'SCHEMA CHANGE GC' AND description LIKE 'GC for DROP TABLE test.public.t_control_job_priv' + +statement error user testuser does not have privileges for job +PAUSE JOB (SELECT $job_id) + +user root + +statement ok +GRANT SYSTEM CONTROLJOB TO testuser + +user testuser + +statement ok +PAUSE JOB (SELECT $job_id) + +user root + +statement ok +REVOKE SYSTEM CONTROLJOB FROM testuser + +subtest end + +subtest control_job_priv_inherited + +user testuser + +statement ok +CREATE TABLE t_control_job_priv_inherited(x INT); +DROP TABLE t_control_job_priv_inherited + +let $job_id +SELECT job_id FROM [SHOW JOBS] WHERE user_name = 'testuser' AND job_type = 'SCHEMA CHANGE GC' AND description LIKE 'GC for DROP TABLE test.public.t_control_job_priv_inherited' + +statement error user testuser does not have privileges for job +PAUSE JOB (SELECT $job_id) + +user root + +statement ok +CREATE ROLE jobcontroller + +statement ok +GRANT SYSTEM CONTROLJOB TO jobcontroller + +statement ok +GRANT jobcontroller TO testuser + +user testuser + +statement ok +PAUSE JOB (SELECT $job_id) + +user root + +statement ok +REVOKE SYSTEM CONTROLJOB FROM jobcontroller + +statement ok +REVOKE jobcontroller FROM testuser + +statement ok +DROP ROLE jobcontroller + +subtest end diff --git a/pkg/sql/privilege/kind_string.go b/pkg/sql/privilege/kind_string.go index ff35bd36bc55..281472f74417 100644 --- a/pkg/sql/privilege/kind_string.go +++ b/pkg/sql/privilege/kind_string.go @@ -42,7 +42,8 @@ func _() { _ = x[CREATEROLE-32] _ = x[CREATELOGIN-33] _ = x[CREATEDB-34] - _ = x[largestKind-34] + _ = x[CONTROLJOB-35] + _ = x[largestKind-35] } func (i Kind) String() string { @@ -115,6 +116,8 @@ func (i Kind) String() string { return "CREATELOGIN" case CREATEDB: return "CREATEDB" + case CONTROLJOB: + return "CONTROLJOB" default: return "Kind(" + strconv.FormatInt(int64(i), 10) + ")" } diff --git a/pkg/sql/privilege/privilege.go b/pkg/sql/privilege/privilege.go index 2fcdafd8dc09..26929eb5e093 100644 --- a/pkg/sql/privilege/privilege.go +++ b/pkg/sql/privilege/privilege.go @@ -75,7 +75,8 @@ const ( CREATEROLE Kind = 32 CREATELOGIN Kind = 33 CREATEDB Kind = 34 - largestKind = CREATEDB + CONTROLJOB Kind = 35 + largestKind = CONTROLJOB ) var isDeprecatedKind = map[Kind]bool{ @@ -163,7 +164,7 @@ var ( GlobalPrivileges = List{ ALL, BACKUP, RESTORE, MODIFYCLUSTERSETTING, EXTERNALCONNECTION, VIEWACTIVITY, VIEWACTIVITYREDACTED, VIEWCLUSTERSETTING, CANCELQUERY, NOSQLLOGIN, VIEWCLUSTERMETADATA, VIEWDEBUG, EXTERNALIOIMPLICITACCESS, VIEWJOB, - MODIFYSQLCLUSTERSETTING, REPLICATION, MANAGETENANT, VIEWSYSTEMTABLE, CREATEROLE, CREATELOGIN, CREATEDB, + MODIFYSQLCLUSTERSETTING, REPLICATION, MANAGETENANT, VIEWSYSTEMTABLE, CREATEROLE, CREATELOGIN, CREATEDB, CONTROLJOB, } VirtualTablePrivileges = List{ALL, SELECT} ExternalConnectionPrivileges = List{ALL, USAGE, DROP}