Skip to content

Commit

Permalink
sql,ui: enable non-admin users to see their own jobs
Browse files Browse the repository at this point in the history
Release note (ui change): Non-admin users can now use the "Jobs
detail" page and see their own jobs.

Release note (sql change): Non-admin users can now query `SHOW JOBS`
and `crdb_internal.jobs` and see their own jobs.
  • Loading branch information
knz committed Jan 27, 2020
1 parent 91bd480 commit 606db1d
Show file tree
Hide file tree
Showing 3 changed files with 125 additions and 31 deletions.
72 changes: 42 additions & 30 deletions pkg/server/admin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1250,14 +1250,16 @@ func TestAdminAPIJobs(t *testing.T) {
status jobs.Status
details jobspb.Details
progress jobspb.ProgressDetails
username string
}{
{1, jobs.StatusRunning, jobspb.RestoreDetails{}, jobspb.RestoreProgress{}},
{2, jobs.StatusRunning, jobspb.BackupDetails{}, jobspb.BackupProgress{}},
{3, jobs.StatusSucceeded, jobspb.BackupDetails{}, jobspb.BackupProgress{}},
{4, jobs.StatusRunning, jobspb.ChangefeedDetails{}, jobspb.ChangefeedProgress{}},
{1, jobs.StatusRunning, jobspb.RestoreDetails{}, jobspb.RestoreProgress{}, security.RootUser},
{2, jobs.StatusRunning, jobspb.BackupDetails{}, jobspb.BackupProgress{}, security.RootUser},
{3, jobs.StatusSucceeded, jobspb.BackupDetails{}, jobspb.BackupProgress{}, security.RootUser},
{4, jobs.StatusRunning, jobspb.ChangefeedDetails{}, jobspb.ChangefeedProgress{}, security.RootUser},
{5, jobs.StatusSucceeded, jobspb.BackupDetails{}, jobspb.BackupProgress{}, authenticatedUserNameNoAdmin},
}
for _, job := range testJobs {
payload := jobspb.Payload{Details: jobspb.WrapPayloadDetails(job.details)}
payload := jobspb.Payload{Username: job.username, Details: jobspb.WrapPayloadDetails(job.details)}
payloadBytes, err := protoutil.Marshal(&payload)
if err != nil {
t.Fatal(err)
Expand Down Expand Up @@ -1289,33 +1291,43 @@ func TestAdminAPIJobs(t *testing.T) {
const invalidJobType = math.MaxInt32

testCases := []struct {
uri string
expectedIDs []int64
uri string
expectedIDsViaAdmin []int64
expectedIDsViaNonAdmin []int64
}{
{"jobs", append([]int64{4, 3, 2, 1}, existingIDs...)},
{"jobs?limit=1", []int64{4}},
{"jobs?status=running", []int64{4, 2, 1}},
{"jobs?status=succeeded", append([]int64{3}, existingIDs...)},
{"jobs?status=pending", []int64{}},
{"jobs?status=garbage", []int64{}},
{fmt.Sprintf("jobs?type=%d", jobspb.TypeBackup), []int64{3, 2}},
{fmt.Sprintf("jobs?type=%d", jobspb.TypeRestore), []int64{1}},
{fmt.Sprintf("jobs?type=%d", invalidJobType), []int64{}},
{fmt.Sprintf("jobs?status=running&type=%d", jobspb.TypeBackup), []int64{2}},
}
for i, testCase := range testCases {
var res serverpb.JobsResponse
if err := getAdminJSONProto(s, testCase.uri, &res); err != nil {
t.Fatal(err)
}
resIDs := []int64{}
for _, job := range res.Jobs {
resIDs = append(resIDs, job.ID)
}
if e, a := testCase.expectedIDs, resIDs; !reflect.DeepEqual(e, a) {
t.Errorf("%d: expected job IDs %v, but got %v", i, e, a)
{"jobs", append([]int64{5, 4, 3, 2, 1}, existingIDs...), []int64{5}},
{"jobs?limit=1", []int64{5}, []int64{5}},
{"jobs?status=running", []int64{4, 2, 1}, []int64{}},
{"jobs?status=succeeded", append([]int64{5, 3}, existingIDs...), []int64{5}},
{"jobs?status=pending", []int64{}, []int64{}},
{"jobs?status=garbage", []int64{}, []int64{}},
{fmt.Sprintf("jobs?type=%d", jobspb.TypeBackup), []int64{5, 3, 2}, []int64{5}},
{fmt.Sprintf("jobs?type=%d", jobspb.TypeRestore), []int64{1}, []int64{}},
{fmt.Sprintf("jobs?type=%d", invalidJobType), []int64{}, []int64{}},
{fmt.Sprintf("jobs?status=running&type=%d", jobspb.TypeBackup), []int64{2}, []int64{}},
}

testutils.RunTrueAndFalse(t, "isAdmin", func(t *testing.T, isAdmin bool) {
for i, testCase := range testCases {
var res serverpb.JobsResponse
if err := getAdminJSONProtoWithAdminOption(s, testCase.uri, &res, isAdmin); err != nil {
t.Fatal(err)
}
resIDs := []int64{}
for _, job := range res.Jobs {
resIDs = append(resIDs, job.ID)
}

expected := testCase.expectedIDsViaAdmin
if !isAdmin {
expected = testCase.expectedIDsViaNonAdmin
}

if e, a := expected, resIDs; !reflect.DeepEqual(e, a) {
t.Errorf("%d: expected job IDs %v, but got %v", i, e, a)
}
}
}
})
}

func TestAdminAPILocations(t *testing.T) {
Expand Down
20 changes: 19 additions & 1 deletion pkg/sql/crdb_internal.go
Original file line number Diff line number Diff line change
Expand Up @@ -453,10 +453,18 @@ CREATE TABLE crdb_internal.jobs (
coordinator_id INT
)`,
populate: func(ctx context.Context, p *planner, _ *DatabaseDescriptor, addRow func(...tree.Datum) error) error {
currentUser := p.SessionData().User
isAdmin, err := p.HasAdminRole(ctx)
if err != nil {
return err
}

// Beware: we're querying system.jobs as root; we need to be careful to filter
// out results that the current user is not able to see.
query := `SELECT id, status, created, payload, progress FROM system.jobs`
rows, err := p.ExtendedEvalContext().ExecCfg.InternalExecutor.QueryEx(
ctx, "crdb-internal-jobs-table", p.txn,
sqlbase.InternalExecutorSessionDataOverride{User: p.SessionData().User},
sqlbase.InternalExecutorSessionDataOverride{User: security.RootUser},
query)
if err != nil {
return err
Expand All @@ -472,6 +480,16 @@ CREATE TABLE crdb_internal.jobs (

// Extract data from the payload.
payload, err := jobs.UnmarshalPayload(payloadBytes)

// We filter out masked rows before we allocate all the
// datums. Needless allocate when not necessary.
sameUser := payload != nil && payload.Username == currentUser
if canAccess := isAdmin || sameUser; !canAccess {
// This user is neither an admin nor the user who created the
// job. They cannot see this row.
continue
}

if err != nil {
errorStr = tree.NewDString(fmt.Sprintf("error decoding payload: %v", err))
} else {
Expand Down
64 changes: 64 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/jobs
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
# These test verify that a user's job are visible via
# crdb_internal.jobs and SHOW JOBS.

user root

statement ok
GRANT ALL ON DATABASE test TO testuser

statement ok
CREATE TABLE t(x INT); INSERT INTO t(x) VALUES (1); CREATE INDEX ON t(x)

query TTT
SELECT job_type, description, user_name FROM [SHOW JOBS]
----
SCHEMA CHANGE CREATE INDEX ON test.public.t (x) root

query TTT
SELECT job_type, description, user_name FROM crdb_internal.jobs
----
SCHEMA CHANGE CREATE INDEX ON test.public.t (x) root

user testuser

# a non-admin user cannot see the admin jobs

query TTT
SELECT job_type, description, user_name FROM [SHOW JOBS]
----

query TTT
SELECT job_type, description, user_name FROM crdb_internal.jobs
----

# However they can see their own jobs.

statement ok
CREATE TABLE u(x INT); INSERT INTO u(x) VALUES (1); CREATE INDEX ON u(x);


query TTT
SELECT job_type, description, user_name FROM [SHOW JOBS]
----
SCHEMA CHANGE CREATE INDEX ON test.public.u (x) testuser

query TTT
SELECT job_type, description, user_name FROM crdb_internal.jobs
----
SCHEMA CHANGE CREATE INDEX ON test.public.u (x) testuser

# And root can see both.

user root

query TTT
SELECT job_type, description, user_name FROM [SHOW JOBS]
----
SCHEMA CHANGE CREATE INDEX ON test.public.t (x) root
SCHEMA CHANGE CREATE INDEX ON test.public.u (x) testuser

query TTT
SELECT job_type, description, user_name FROM crdb_internal.jobs
----
SCHEMA CHANGE CREATE INDEX ON test.public.t (x) root
SCHEMA CHANGE CREATE INDEX ON test.public.u (x) testuser

0 comments on commit 606db1d

Please sign in to comment.