sql,ui: enable non-admin users to see their own jobs

[knz]

Informs #44335.

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.

[cockroach-teamcity]

This change is 

[knz]

(Will add the missing tests once CI tells me what I need to change.)

[spaskob]

I don't feel to qualified to review on my own, please contact @dt as well

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @knz and @spaskob)

---

pkg/sql/crdb_internal.go, line 463 at r1 (raw file):

		// 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.

@dt how does this interplay with you work on cloud keys

---

pkg/sql/crdb_internal.go, line 486 at r1 (raw file):

			// We filter out masked rows before we allocate all the
			// datums. Needless allocate when not necessary.
			if !(isAdmin || (payload != nil && payload.Username == currentUser)) {

I find it hard to read and reason that is correct; are you sure it's not ... && ...
I feel like this may need a test as well.

[dt]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @dt and @spaskob)

---

pkg/sql/crdb_internal.go, line 463 at r1 (raw file):

Previously, spaskob (Spas Bojanov) wrote…

@dt how does this interplay with you work on cloud keys

If the user was an admin, they could already read secrets/keys directly from system.jobs, and it if is their own job, then it is their own keys anyway (account-sharing notwithstanding). Overall I don't think this significantly changes anything w.r.t. secrets stored in job payloads.

Additionally, we only show them the payload.Description which typically hides they secrets, but I'd rather not include that here -- the hiding is only superficial since the secrets are still in and need to be in the job itself, so pretending they're gone clouds some people's thread model, but that's a separate issue.

---

pkg/sql/crdb_internal.go, line 486 at r1 (raw file):

Previously, spaskob (Spas Bojanov) wrote…

I find it hard to read and reason that is correct; are you sure it's not ... && ...
I feel like this may need a test as well.

even just binding an intermediate bool and moving the extra inversion could help IMO:
if canSeeJob := isAdmin || (paylod!=nil && payload.Username == currentUser); !canSeeJob { continue }

[knz]

Added the missing test cases. PTAL

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @spaskob)

---

pkg/sql/crdb_internal.go, line 463 at r1 (raw file):

Previously, dt (David Taylor) wrote…

If the user was an admin, they could already read secrets/keys directly from system.jobs, and it if is their own job, then it is their own keys anyway (account-sharing notwithstanding). Overall I don't think this significantly changes anything w.r.t. secrets stored in job payloads.

Additionally, we only show them the payload.Description which typically hides they secrets, but I'd rather not include that here -- the hiding is only superficial since the secrets are still in and need to be in the job itself, so pretending they're gone clouds some people's thread model, but that's a separate issue.

Thanks for checking

---

pkg/sql/crdb_internal.go, line 486 at r1 (raw file):

Previously, dt (David Taylor) wrote…

even just binding an intermediate bool and moving the extra inversion could help IMO:
if canSeeJob := isAdmin || (paylod!=nil && payload.Username == currentUser); !canSeeJob { continue }

Done.

[spaskob]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @knz and @spaskob)

---

pkg/sql/crdb_internal.go, line 488 at r2 (raw file):

			sameUser := payload != nil && payload.Username == currentUser
			canAccess := isAdmin || sameUser
			if !canAccess {

nit: if canAccess := isAdmin || sameUser; !canAccess {

[knz]

TFYR!

bors r=dt,spaskob

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @spaskob)

---

pkg/sql/crdb_internal.go, line 488 at r2 (raw file):

Previously, spaskob (Spas Bojanov) wrote…

nit: if canAccess := isAdmin || sameUser; !canAccess {

Done.

[craig]

Build succeeded

• GitHub CI (Cockroach)