Skip to content

Commit 5ba7ba6

Browse files
mafredrimafredri
authored
fix(coderd): add strict org ID joins for provisioner job metadata (#16588)
References #16558
1 parent 46e04c6 commit 5ba7ba6

File tree

6 files changed

+69
-21
lines changed

6 files changed

+69
-21
lines changed

‎coderd/database/dbauthz/dbauthz_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3354,11 +3354,11 @@ func (s *MethodTestSuite) TestExtraMethods() {
33543354
dbgen.WorkspaceBuild(s.T(), db, database.WorkspaceBuild{ID: wbID, WorkspaceID: w.ID, TemplateVersionID: tv.ID, JobID: j2.ID})
33553355

33563356
ds, err := db.GetProvisionerJobsByOrganizationAndStatusWithQueuePositionAndProvisioner(context.Background(), database.GetProvisionerJobsByOrganizationAndStatusWithQueuePositionAndProvisionerParams{
3357-
OrganizationID: uuid.NullUUID{Valid: true, UUID: org.ID},
3357+
OrganizationID: org.ID,
33583358
})
33593359
s.NoError(err, "get provisioner jobs by org")
33603360
check.Args(database.GetProvisionerJobsByOrganizationAndStatusWithQueuePositionAndProvisionerParams{
3361-
OrganizationID: uuid.NullUUID{Valid: true, UUID: org.ID},
3361+
OrganizationID: org.ID,
33623362
}).Asserts(j1, policy.ActionRead, j2, policy.ActionRead).Returns(ds)
33633363
}))
33643364
}

‎coderd/database/dbmem/dbmem.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4221,7 +4221,7 @@ func (q *FakeQuerier) GetProvisionerJobsByOrganizationAndStatusWithQueuePosition
42214221
for _, rowQP := range rowsWithQueuePosition {
42224222
job := rowQP.ProvisionerJob
42234223

4224-
if arg.OrganizationID.Valid && job.OrganizationID != arg.OrganizationID.UUID {
4224+
if job.OrganizationID != arg.OrganizationID {
42254225
continue
42264226
}
42274227
if len(arg.Status) > 0 && !slices.Contains(arg.Status, job.JobStatus) {

‎coderd/database/queries.sql.go

Lines changed: 33 additions & 9 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries/provisionerdaemons.sql

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ JOIN
5858
LEFT JOIN
5959
provisioner_jobs current_job ON (
6060
current_job.worker_id = pd.id
61+
AND current_job.organization_id = pd.organization_id
6162
AND current_job.completed_at IS NULL
6263
)
6364
LEFT JOIN
@@ -69,28 +70,42 @@ LEFT JOIN
6970
provisioner_jobs
7071
WHERE
7172
worker_id = pd.id
73+
AND organization_id = pd.organization_id
7274
AND completed_at IS NOT NULL
7375
ORDER BY
7476
completed_at DESC
7577
LIMIT 1
7678
)
79+
AND previous_job.organization_id = pd.organization_id
7780
)
7881
-- Current job information.
7982
LEFT JOIN
8083
workspace_builds current_build ON current_build.id = CASE WHEN current_job.input ? 'workspace_build_id' THEN (current_job.input->>'workspace_build_id')::uuid END
8184
LEFT JOIN
8285
-- We should always have a template version, either explicitly or implicitly via workspace build.
83-
template_versions current_version ON current_version.id = CASE WHEN current_job.input ? 'template_version_id' THEN (current_job.input->>'template_version_id')::uuid ELSE current_build.template_version_id END
86+
template_versions current_version ON (
87+
current_version.id = CASE WHEN current_job.input ? 'template_version_id' THEN (current_job.input->>'template_version_id')::uuid ELSE current_build.template_version_id END
88+
AND current_version.organization_id = pd.organization_id
89+
)
8490
LEFT JOIN
85-
templates current_template ON current_template.id = current_version.template_id
91+
templates current_template ON (
92+
current_template.id = current_version.template_id
93+
AND current_template.organization_id = pd.organization_id
94+
)
8695
-- Previous job information.
8796
LEFT JOIN
8897
workspace_builds previous_build ON previous_build.id = CASE WHEN previous_job.input ? 'workspace_build_id' THEN (previous_job.input->>'workspace_build_id')::uuid END
8998
LEFT JOIN
9099
-- We should always have a template version, either explicitly or implicitly via workspace build.
91-
template_versions previous_version ON previous_version.id = CASE WHEN previous_job.input ? 'template_version_id' THEN (previous_job.input->>'template_version_id')::uuid ELSE previous_build.template_version_id END
100+
template_versions previous_version ON (
101+
previous_version.id = CASE WHEN previous_job.input ? 'template_version_id' THEN (previous_job.input->>'template_version_id')::uuid ELSE previous_build.template_version_id END
102+
AND previous_version.organization_id = pd.organization_id
103+
)
92104
LEFT JOIN
93-
templates previous_template ON previous_template.id = previous_version.template_id
105+
templates previous_template ON (
106+
previous_template.id = previous_version.template_id
107+
AND previous_template.organization_id = pd.organization_id
108+
)
94109
WHERE
95110
pd.organization_id = @organization_id::uuid
96111
AND (COALESCE(array_length(@ids::uuid[], 1), 0) = 0 OR pd.id = ANY(@ids::uuid[]))

‎coderd/database/queries/provisionerjobs.sql

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -148,14 +148,23 @@ LEFT JOIN
148148
LEFT JOIN
149149
workspace_builds wb ON wb.id = CASE WHEN pj.input ? 'workspace_build_id' THEN (pj.input->>'workspace_build_id')::uuid END
150150
LEFT JOIN
151-
workspaces w ON wb.workspace_id = w.id
151+
workspaces w ON (
152+
w.id = wb.workspace_id
153+
AND w.organization_id = pj.organization_id
154+
)
152155
LEFT JOIN
153156
-- We should always have a template version, either explicitly or implicitly via workspace build.
154-
template_versions tv ON tv.id = CASE WHEN pj.input ? 'template_version_id' THEN (pj.input->>'template_version_id')::uuid ELSE wb.template_version_id END
157+
template_versions tv ON (
158+
tv.id = CASE WHEN pj.input ? 'template_version_id' THEN (pj.input->>'template_version_id')::uuid ELSE wb.template_version_id END
159+
AND tv.organization_id = pj.organization_id
160+
)
155161
LEFT JOIN
156-
templates t ON tv.template_id = t.id
162+
templates t ON (
163+
t.id = tv.template_id
164+
AND t.organization_id = pj.organization_id
165+
)
157166
WHERE
158-
(sqlc.narg('organization_id')::uuid IS NULL OR pj.organization_id = @organization_id)
167+
pj.organization_id = @organization_id::uuid
159168
AND (COALESCE(array_length(@ids::uuid[], 1), 0) = 0 OR pj.id = ANY(@ids::uuid[]))
160169
AND (COALESCE(array_length(@status::provisioner_job_status[], 1), 0) = 0 OR pj.job_status = ANY(@status::provisioner_job_status[]))
161170
AND (@tags::tagset = 'null'::tagset OR provisioner_tagset_contains(pj.tags::tagset, @tags::tagset))

‎coderd/provisionerjobs.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ func (api *API) handleAuthAndFetchProvisionerJobs(rw http.ResponseWriter, r *htt
130130
}
131131

132132
jobs, err := api.Database.GetProvisionerJobsByOrganizationAndStatusWithQueuePositionAndProvisioner(ctx, database.GetProvisionerJobsByOrganizationAndStatusWithQueuePositionAndProvisionerParams{
133-
OrganizationID: uuid.NullUUID{UUID: org.ID, Valid: true},
133+
OrganizationID: org.ID,
134134
Status: slice.StringEnums[database.ProvisionerJobStatus](status),
135135
Limit: sql.NullInt32{Int32: limit, Valid: limit > 0},
136136
IDs: ids,

0 commit comments

Comments
 (0)