jobs: add VIEWJOB global privilege, remove role option

This change updates `VIEWJOB` to be a global privilege instead
of a role option so that it can be inherited from roles to
their members.

Previously, `VIEWJOB` was a role option which could be granted to users.
Now, `VIEWJOB` is a global privilege. Granting this privilege to a user
or role has the syntax `GRANT SYSTEM VIEWJOB TO user`. Using `VIEWJOB`
as a role option is deprecated.

Note that the `VIEWJOB` role option was not included in any release so far.
It was queued up to be released in 23.1, but was not. This change
is also being queued for 23.1, so there should not be any backwards
compatibility issues.

Informs: cockroachdb#96382
Epic: none

docs/generated/sql/bnf/stmt_block.bnf

@@ -1249,7 +1249,6 @@ unreserved_keyword ::=
 	| 'NOVIEWACTIVITY'
 	| 'NOVIEWACTIVITYREDACTED'
 	| 'NOVIEWCLUSTERSETTING'
-	| 'NOVIEWJOB'
 	| 'NOWAIT'
 	| 'NULLS'
 	| 'IGNORE_FOREIGN_KEYS'
@@ -1444,7 +1443,6 @@ unreserved_keyword ::=
 	| 'VIEWCLUSTERMETADATA'
 	| 'VIEWCLUSTERSETTING'
 	| 'VIEWDEBUG'
-	| 'VIEWJOB'
 	| 'VISIBLE'
 	| 'VOLATILE'
 	| 'VOTERS'
@@ -2964,8 +2962,6 @@ role_option ::=
 	| 'NOSQLLOGIN'
 	| 'VIEWCLUSTERSETTING'
 	| 'NOVIEWCLUSTERSETTING'
-	| 'VIEWJOB'
-	| 'NOVIEWJOB'
 	| password_clause
 	| valid_until_clause
 

pkg/jobs/jobsauth/BUILD.bazel

@@ -14,6 +14,7 @@ go_library(
         "//pkg/sql/pgwire/pgerror",
         "//pkg/sql/privilege",
         "//pkg/sql/roleoption",
+        "//pkg/sql/syntheticprivilege",
     ],
 )
 

pkg/jobs/jobsauth/authorization.go

@@ -21,6 +21,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"
 )
 
 // An AccessLevel is used to indicate how strict an authorization check should
@@ -57,6 +58,9 @@ type AuthorizationAccessor interface {
 	// CheckPrivilegeForTableID mirrors sql.AuthorizationAccessor.
 	CheckPrivilegeForTableID(ctx context.Context, tableID descpb.ID, privilege privilege.Kind) error
 
+	// HasPrivilege mirrors sql.AuthorizationAccessor.
+	HasPrivilege(ctx context.Context, privilegeObject privilege.Object, privilege privilege.Kind, user username.SQLUsername) (bool, error)
+
 	// HasRoleOption mirrors sql.AuthorizationAccessor.
 	HasRoleOption(ctx context.Context, roleOption roleoption.Option) (bool, error)
 
@@ -102,7 +106,7 @@ func Authorize(
 		return err
 	}
 
-	hasViewJob, err := a.HasRoleOption(ctx, roleoption.VIEWJOB)
+	hasViewJob, err := a.HasPrivilege(ctx, &syntheticprivilege.GlobalPrivilege{}, privilege.VIEWJOB, a.User())
 	if err != nil {
 		return err
 	}
@@ -132,6 +136,6 @@ func Authorize(
 		return check(ctx, a, jobID, payload)
 	}
 	return pgerror.Newf(pgcode.InsufficientPrivilege,
-		"user %s does not have %s privilege for job $d",
-		a.User(), roleoption.CONTROLJOB, jobID)
+		"user %s does not have privileges for job $d",
+		a.User(), jobID)
 }

pkg/jobs/jobsauth/authorization_test.go

@@ -12,6 +12,7 @@ package jobsauth_test
 
 import (
 	"context"
+	"fmt"
 	"math/rand"
 	"testing"
 
@@ -29,12 +30,22 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
+type userPrivilege struct {
+	privilege.Kind
+	privilege.ObjectType
+}
+
+var viewJobGlobalPrivilege = userPrivilege{privilege.VIEWJOB, privilege.Global}
+
 type testAuthAccessor struct {
 	user username.SQLUsername
 
 	// set of role options that the user has
 	roleOptions map[roleoption.Option]struct{}
 
+	// set of privileges that the user has
+	userPrivileges map[userPrivilege]struct{}
+
 	// set of descriptors which the user has privilege.CHANGEFEED on
 	changeFeedPrivileges map[descpb.ID]struct{}
 	// set of descriptors which are dropped
@@ -65,6 +76,23 @@ func (a *testAuthAccessor) CheckPrivilegeForTableID(
 	return nil
 }
 
+// HasPrivilege is only implemented for the target user.
+func (a *testAuthAccessor) HasPrivilege(
+	ctx context.Context,
+	privilegeObject privilege.Object,
+	privilegeKind privilege.Kind,
+	sqlUsername username.SQLUsername,
+) (bool, error) {
+	if sqlUsername != a.user {
+		panic(fmt.Sprintf("testAuthAccessor does not implement HasPrivilege for user %s", sqlUsername))
+	}
+	_, ok := a.userPrivileges[userPrivilege{privilegeKind, privilegeObject.GetObjectType()}]
+	if ok {
+		return true, nil
+	}
+	return false, nil
+}
+
 func (a *testAuthAccessor) HasRoleOption(
 	_ context.Context, roleOption roleoption.Option,
 ) (bool, error) {
@@ -119,6 +147,7 @@ func TestAuthorization(t *testing.T) {
 
 		user                 username.SQLUsername
 		roleOptions          map[roleoption.Option]struct{}
+		userPrivileges       map[userPrivilege]struct{}
 		changeFeedPrivileges map[descpb.ID]struct{}
 		droppedDescriptors   map[descpb.ID]struct{}
 		admins               map[string]struct{}
@@ -206,19 +235,28 @@ func TestAuthorization(t *testing.T) {
 			accessLevel: jobsauth.ControlAccess,
 		},
 		{
-			name:        "viewjob-allows-read-access",
-			user:        username.MakeSQLUsernameFromPreNormalizedString("user1"),
-			roleOptions: map[roleoption.Option]struct{}{roleoption.VIEWJOB: {}},
-			admins:      map[string]struct{}{},
+			name:   "viewjob-required-for-read-access",
+			user:   username.MakeSQLUsernameFromPreNormalizedString("user1"),
+			admins: map[string]struct{}{},
 
 			payload:     makeBackupPayload("user2"),
 			accessLevel: jobsauth.ViewAccess,
+			userErr:     pgerror.New(pgcode.InsufficientPrivilege, "foo"),
 		},
 		{
-			name:        "viewjob-disallows-control-access",
-			user:        username.MakeSQLUsernameFromPreNormalizedString("user1"),
-			roleOptions: map[roleoption.Option]struct{}{roleoption.VIEWJOB: {}},
-			admins:      map[string]struct{}{},
+			name:           "viewjob-allows-read-access",
+			user:           username.MakeSQLUsernameFromPreNormalizedString("user1"),
+			userPrivileges: map[userPrivilege]struct{}{viewJobGlobalPrivilege: {}},
+			admins:         map[string]struct{}{},
+
+			payload:     makeBackupPayload("user2"),
+			accessLevel: jobsauth.ViewAccess,
+		},
+		{
+			name:           "viewjob-disallows-control-access",
+			user:           username.MakeSQLUsernameFromPreNormalizedString("user1"),
+			userPrivileges: map[userPrivilege]struct{}{viewJobGlobalPrivilege: {}},
+			admins:         map[string]struct{}{},
 
 			payload:     makeBackupPayload("user2"),
 			accessLevel: jobsauth.ControlAccess,
@@ -229,6 +267,7 @@ func TestAuthorization(t *testing.T) {
 			testAuth := &testAuthAccessor{
 				user:                 tc.user,
 				roleOptions:          tc.roleOptions,
+				userPrivileges:       tc.userPrivileges,
 				changeFeedPrivileges: tc.changeFeedPrivileges,
 				droppedDescriptors:   tc.droppedDescriptors,
 				admins:               tc.admins,

pkg/sql/logictest/testdata/logic_test/jobs

@@ -173,19 +173,19 @@ SELECT job_id FROM [SHOW JOBS] WHERE user_name = 'testuser2' AND job_type = 'SCH
 user testuser
 
 # testuser should no longer have the ability to control jobs.
-statement error pq: user testuser does not have CONTROLJOB privilege
+statement error pq: user testuser does not have privileges for job
 PAUSE JOB (SELECT $job_id)
 
-statement error pq: user testuser does not have CONTROLJOB privilege
+statement error pq: user testuser does not have privileges for job
 CANCEL JOB (SELECT $job_id)
 
-statement error pq: user testuser does not have CONTROLJOB privilege
+statement error pq: user testuser does not have privileges for job
 RESUME JOB (SELECT $job_id)
 
 user root
 
 statement ok
-ALTER ROLE testuser VIEWJOB
+GRANT SYSTEM VIEWJOB TO testuser
 
 user testuser
 
@@ -202,7 +202,42 @@ SCHEMA CHANGE GC  GC for temporary index used during index backfill  testuser2
 user root
 
 statement ok
-ALTER ROLE testuser NOVIEWJOB
+REVOKE SYSTEM VIEWJOB FROM testuser
+
+user testuser
+
+# testuser can only see their own job
+query TTT
+SELECT job_type, description, user_name FROM crdb_internal.jobs WHERE job_type = 'SCHEMA CHANGE GC' ORDER BY 1, 2, 3
+----
+SCHEMA CHANGE GC  GC for temporary index used during index backfill  testuser
+
+user root
+
+statement ok
+CREATE ROLE jobviewer
+
+statement ok
+GRANT SYSTEM VIEWJOB TO jobviewer
+
+statement ok
+GRANT jobviewer TO testuser
+
+user testuser
+
+# testuser should be able to see all jobs
+query TTT
+SELECT job_type, description, user_name FROM crdb_internal.jobs WHERE job_type = 'SCHEMA CHANGE GC' ORDER BY 1, 2, 3
+----
+SCHEMA CHANGE GC  GC for DROP TABLE test.public.t1                   testuser2
+SCHEMA CHANGE GC  GC for DROP TABLE test.public.t2                   root
+SCHEMA CHANGE GC  GC for temporary index used during index backfill  root
+SCHEMA CHANGE GC  GC for temporary index used during index backfill  testuser
+SCHEMA CHANGE GC  GC for temporary index used during index backfill  testuser2
+
+user root
+statement ok
+REVOKE jobviewer FROM testuser
 
 user testuser
 

pkg/sql/logictest/testdata/logic_test/role

@@ -918,15 +918,6 @@ ALTER ROLE testrole WITH CREATEROLE CREATEROLE
 statement error pq: redundant role options
 ALTER ROLE testrole WITH NOCREATEROLE NOCREATEROLE
 
-statement error pq: conflicting role options
-ALTER ROLE testrole WITH VIEWJOB NOVIEWJOB
-
-statement error pq: redundant role options
-ALTER ROLE testrole WITH VIEWJOB VIEWJOB
-
-statement error pq: redundant role options
-ALTER ROLE testrole WITH NOVIEWJOB NOVIEWJOB
-
 statement ok
 CREATE ROLE rolewithcreate WITH CREATEROLE
 

pkg/sql/parser/sql.y

@@ -948,7 +948,7 @@ func (u *sqlSymUnion) showTenantOpts() tree.ShowTenantOptions {
 %token <str> NOSQLLOGIN NO_INDEX_JOIN NO_ZIGZAG_JOIN NO_FULL_SCAN NONE NONVOTERS NORMAL NOT
 %token <str> NOTHING NOTHING_AFTER_RETURNING
 %token <str> NOTNULL
-%token <str> NOVIEWACTIVITY NOVIEWACTIVITYREDACTED NOVIEWCLUSTERSETTING NOVIEWJOB NOWAIT NULL NULLIF NULLS NUMERIC
+%token <str> NOVIEWACTIVITY NOVIEWACTIVITYREDACTED NOVIEWCLUSTERSETTING NOWAIT NULL NULLIF NULLS NUMERIC
 
 %token <str> OF OFF OFFSET OID OIDS OIDVECTOR OLD_KMS ON ONLY OPT OPTION OPTIONS OR
 %token <str> ORDER ORDINALITY OTHERS OUT OUTER OVER OVERLAPS OVERLAY OWNED OWNER OPERATOR
@@ -985,7 +985,7 @@ func (u *sqlSymUnion) showTenantOpts() tree.ShowTenantOptions {
 %token <str> UPDATE UPSERT UNSET UNTIL USE USER USERS USING UUID
 
 %token <str> VALID VALIDATE VALUE VALUES VARBIT VARCHAR VARIADIC VERIFY_BACKUP_TABLE_DATA VIEW VARYING VIEWACTIVITY VIEWACTIVITYREDACTED VIEWDEBUG
-%token <str> VIEWCLUSTERMETADATA VIEWCLUSTERSETTING VIEWJOB VIRTUAL VISIBLE INVISIBLE VOLATILE VOTERS
+%token <str> VIEWCLUSTERMETADATA VIEWCLUSTERSETTING VIRTUAL VISIBLE INVISIBLE VOLATILE VOTERS
 
 %token <str> WHEN WHERE WINDOW WITH WITHIN WITHOUT WORK WRITE
 
@@ -10320,14 +10320,6 @@ role_option:
   {
     $$.val = tree.KVOption{Key: tree.Name($1), Value: nil}
   }
-| VIEWJOB
-  {
-    $$.val = tree.KVOption{Key: tree.Name($1), Value: nil}
-  }
-| NOVIEWJOB
-  {
-    $$.val = tree.KVOption{Key: tree.Name($1), Value: nil}
-  }
 | password_clause
 | valid_until_clause
 
@@ -16356,7 +16348,6 @@ unreserved_keyword:
 | NOVIEWACTIVITY
 | NOVIEWACTIVITYREDACTED
 | NOVIEWCLUSTERSETTING
-| NOVIEWJOB
 | NOWAIT
 | NULLS
 | IGNORE_FOREIGN_KEYS
@@ -16551,7 +16542,6 @@ unreserved_keyword:
 | VIEWCLUSTERMETADATA
 | VIEWCLUSTERSETTING
 | VIEWDEBUG
-| VIEWJOB
 | VISIBLE
 | VOLATILE
 | VOTERS

pkg/sql/privilege/privilege.go

@@ -59,6 +59,7 @@ const (
 	RESTORE                  Kind = 24
 	EXTERNALIOIMPLICITACCESS Kind = 25
 	CHANGEFEED               Kind = 26
+	VIEWJOB                  Kind = 27
 )
 
 // Privilege represents a privilege parsed from an Access Privilege Inquiry
@@ -112,7 +113,7 @@ var isDescriptorBacked = map[ObjectType]bool{
 
 // Predefined sets of privileges.
 var (
-	AllPrivileges         = List{ALL, CHANGEFEED, CONNECT, CREATE, DROP, SELECT, INSERT, DELETE, UPDATE, USAGE, ZONECONFIG, EXECUTE, BACKUP, RESTORE, EXTERNALIOIMPLICITACCESS}
+	AllPrivileges         = List{ALL, CHANGEFEED, CONNECT, CREATE, DROP, SELECT, INSERT, DELETE, UPDATE, USAGE, ZONECONFIG, EXECUTE, BACKUP, RESTORE, EXTERNALIOIMPLICITACCESS, VIEWJOB}
 	ReadData              = List{SELECT}
 	ReadWriteData         = List{SELECT, INSERT, DELETE, UPDATE}
 	ReadWriteSequenceData = List{SELECT, UPDATE, USAGE}
@@ -126,7 +127,7 @@ var (
 	// certain privileges unavailable after upgrade migration.
 	// Note that "CREATE, CHANGEFEED, INSERT, DELETE, ZONECONFIG" are no-op privileges on sequences.
 	SequencePrivileges           = List{ALL, USAGE, SELECT, UPDATE, CREATE, CHANGEFEED, DROP, INSERT, DELETE, ZONECONFIG}
-	GlobalPrivileges             = List{ALL, BACKUP, RESTORE, MODIFYCLUSTERSETTING, EXTERNALCONNECTION, VIEWACTIVITY, VIEWACTIVITYREDACTED, VIEWCLUSTERSETTING, CANCELQUERY, NOSQLLOGIN, VIEWCLUSTERMETADATA, VIEWDEBUG, EXTERNALIOIMPLICITACCESS}
+	GlobalPrivileges             = List{ALL, BACKUP, RESTORE, MODIFYCLUSTERSETTING, EXTERNALCONNECTION, VIEWACTIVITY, VIEWACTIVITYREDACTED, VIEWCLUSTERSETTING, CANCELQUERY, NOSQLLOGIN, VIEWCLUSTERMETADATA, VIEWDEBUG, EXTERNALIOIMPLICITACCESS, VIEWJOB}
 	VirtualTablePrivileges       = List{ALL, SELECT}
 	ExternalConnectionPrivileges = List{ALL, USAGE, DROP}
 )
@@ -168,6 +169,7 @@ var ByName = map[string]Kind{
 	"BACKUP":                   BACKUP,
 	"RESTORE":                  RESTORE,
 	"EXTERNALIOIMPLICITACCESS": EXTERNALIOIMPLICITACCESS,
+	"VIEWJOB":                  VIEWJOB,
 }
 
 // List is a list of privileges.