adjusted TODOs

coder · ibetitsmike · May 20, 2025 · Apr 1, 2025 · May 9, 2025 · May 9, 2025
commit 767cb77cf4bdac0827cf9b90a9e781d3bb6f32b4
diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go
@@ -2320,25 +2320,27 @@ func (q *querier) GetProvisionerJobsByIDs(ctx context.Context, ids []uuid.UUID)
 	if err != nil {
 		return nil, err
 	}
+	orgIDs := make(map[uuid.UUID]struct{})
 	for _, job := range provisionerJobs {
-		if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceProvisionerJobs.InOrg(job.OrganizationID)); err != nil {
+		orgIDs[job.OrganizationID] = struct{}{}
+	}
+	for orgID := range orgIDs {
+		if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceProvisionerJobs.InOrg(orgID)); err != nil {
 			return nil, err
 		}
 	}
 	return provisionerJobs, nil
 }
 
 func (q *querier) GetProvisionerJobsByIDsWithQueuePosition(ctx context.Context, ids []uuid.UUID) ([]database.GetProvisionerJobsByIDsWithQueuePositionRow, error) {
-	// if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceProvisionerJobs); err != nil {
-	// 	return nil, err
-	// }
+	// TODO: Remove this once we have a proper rbac check for provisioner jobs.
+	// Details in https://github.com/coder/coder/issues/16160
 	return q.db.GetProvisionerJobsByIDsWithQueuePosition(ctx, ids)
 }
 
 func (q *querier) GetProvisionerJobsByOrganizationAndStatusWithQueuePositionAndProvisioner(ctx context.Context, arg database.GetProvisionerJobsByOrganizationAndStatusWithQueuePositionAndProvisionerParams) ([]database.GetProvisionerJobsByOrganizationAndStatusWithQueuePositionAndProvisionerRow, error) {
-	// if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceProvisionerJobs); err != nil {
-	// 	return nil, err
-	// }
+	// TODO: Remove this once we have a proper rbac check for provisioner jobs.
+	// Details in https://github.com/coder/coder/issues/16160
 	return fetchWithPostFilter(q.auth, policy.ActionRead, q.db.GetProvisionerJobsByOrganizationAndStatusWithQueuePositionAndProvisioner)(ctx, arg)
 }
 
@@ -3547,9 +3549,8 @@ func (q *querier) InsertProvisionerJob(ctx context.Context, arg database.InsertP
 }
 
 func (q *querier) InsertProvisionerJobLogs(ctx context.Context, arg database.InsertProvisionerJobLogsParams) ([]database.ProvisionerJobLog, error) {
-	// if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceProvisionerJobs); err != nil {
-	// 	return nil, err
-	// }
+	// TODO: Remove this once we have a proper rbac check for provisioner jobs.
+	// Details in https://github.com/coder/coder/issues/16160
 	return q.db.InsertProvisionerJobLogs(ctx, arg)
 }
 
@@ -4187,9 +4188,8 @@ func (q *querier) UpdateProvisionerJobByID(ctx context.Context, arg database.Upd
 }
 
 func (q *querier) UpdateProvisionerJobWithCancelByID(ctx context.Context, arg database.UpdateProvisionerJobWithCancelByIDParams) error {
-	// if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceProvisionerJobs); err != nil {
-	// 	return err
-	// }
+	// TODO: Remove this once we have a proper rbac check for provisioner jobs.
+	// Details in https://github.com/coder/coder/issues/16160
 
 	job, err := q.db.GetProvisionerJobByID(ctx, arg.ID)
 	if err != nil {

diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go
@@ -698,7 +698,7 @@ func (s *MethodTestSuite) TestProvisionerJob() {
 		a := dbgen.ProvisionerJob(s.T(), db, nil, database.ProvisionerJob{OrganizationID: o.ID})
 		b := dbgen.ProvisionerJob(s.T(), db, nil, database.ProvisionerJob{OrganizationID: o.ID})
 		check.Args([]uuid.UUID{a.ID, b.ID}).
-			Asserts(rbac.ResourceProvisionerJobs.InOrg(o.ID), policy.ActionRead, rbac.ResourceProvisionerJobs.InOrg(o.ID), policy.ActionRead).
+			Asserts(rbac.ResourceProvisionerJobs.InOrg(o.ID), policy.ActionRead).
 			Returns(slice.New(a, b))
 	}))
 	s.Run("GetProvisionerLogsAfterID", s.Subtest(func(db database.Store, check *expects) {
@@ -3982,7 +3982,7 @@ func (s *MethodTestSuite) TestSystemFunctions() {
 		a := dbgen.ProvisionerJob(s.T(), db, nil, database.ProvisionerJob{OrganizationID: o.ID})
 		b := dbgen.ProvisionerJob(s.T(), db, nil, database.ProvisionerJob{OrganizationID: o.ID})
 		check.Args([]uuid.UUID{a.ID, b.ID}).
-			Asserts(rbac.ResourceProvisionerJobs.InOrg(o.ID), policy.ActionRead, rbac.ResourceProvisionerJobs.InOrg(o.ID), policy.ActionRead).
+			Asserts(rbac.ResourceProvisionerJobs.InOrg(o.ID), policy.ActionRead).
 			Returns(slice.New(a, b))
 	}))
 	s.Run("InsertWorkspaceAgent", s.Subtest(func(db database.Store, check *expects) {

diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go
@@ -8,7 +8,7 @@ import (
 	"errors"
 	"fmt"
 	"math"
-	"math/rand" //#nosec // this is only used for shuffling an array to pick random jobs to reap
+	insecurerand "math/rand" //#nosec // this is only used for shuffling an array to pick random jobs to reap
 	"reflect"
 	"regexp"
 	"slices"
@@ -4889,7 +4889,7 @@ func (q *FakeQuerier) GetProvisionerJobsToBeReaped(_ context.Context, arg databa
 			}
 		}
 	}
-	rand.Shuffle(len(hungJobs), func(i, j int) {
+	insecurerand.Shuffle(len(hungJobs), func(i, j int) {
 		hungJobs[i], hungJobs[j] = hungJobs[j], hungJobs[i]
 	})
 	return hungJobs, nil