dbauthz: move GetUsersByIDs out of system, modify RBAC check to ResourceUser

johnstcn · johnstcn · commit e5a7cad2b5e0 · 2023-03-09T14:10:34.000Z
diff --git a/coderd/database/dbauthz/querier.go b/coderd/database/dbauthz/querier.go
@@ -963,6 +963,18 @@ func (q *querier) GetUserByID(ctx context.Context, id uuid.UUID) (database.User,
 	return fetch(q.log, q.auth, q.db.GetUserByID)(ctx, id)
 }
 
+// GetUsersByIDs is only used for usernames on workspace return data.
+// This function should be replaced by joining this data to the workspace query
+// itself.
+func (q *querier) GetUsersByIDs(ctx context.Context, ids []uuid.UUID) ([]database.User, error) {
+	for _, uid := range ids {
+		if err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceUser.WithID(uid)); err != nil {
+			return nil, err
+		}
+	}
+	return q.db.GetUsersByIDs(ctx, ids)
+}
+
 func (q *querier) GetAuthorizedUserCount(ctx context.Context, arg database.GetFilteredUserCountParams, prepared rbac.PreparedAuthorized) (int64, error) {
 	return q.db.GetAuthorizedUserCount(ctx, arg, prepared)
 }
diff --git a/coderd/database/dbauthz/querier_test.go b/coderd/database/dbauthz/querier_test.go
@@ -763,6 +763,13 @@ func (s *MethodTestSuite) TestUser() {
 		u := dbgen.User(s.T(), db, database.User{})
 		check.Args(u.ID).Asserts(u, rbac.ActionRead).Returns(u)
 	}))
+	s.Run("GetUsersByIDs", s.Subtest(func(db database.Store, check *expects) {
+		a := dbgen.User(s.T(), db, database.User{CreatedAt: database.Now().Add(-time.Hour)})
+		b := dbgen.User(s.T(), db, database.User{CreatedAt: database.Now()})
+		check.Args([]uuid.UUID{a.ID, b.ID}).
+			Asserts(a, rbac.ActionRead, b, rbac.ActionRead).
+			Returns(slice.New(a, b))
+	}))
 	s.Run("GetAuthorizedUserCount", s.Subtest(func(db database.Store, check *expects) {
 		_ = dbgen.User(s.T(), db, database.User{})
 		check.Args(database.GetFilteredUserCountParams{}, emptyPreparedAuthorized{}).Asserts().Returns(int64(1))
diff --git a/coderd/database/dbauthz/system.go b/coderd/database/dbauthz/system.go
@@ -36,17 +36,6 @@ func (q *querier) GetWorkspaceResourceMetadataByResourceIDs(ctx context.Context,
 	}
 	return q.db.GetWorkspaceResourceMetadataByResourceIDs(ctx, ids)
 }
-
-// GetUsersByIDs is only used for usernames on workspace return data.
-// This function should be replaced by joining this data to the workspace query
-// itself.
-func (q *querier) GetUsersByIDs(ctx context.Context, ids []uuid.UUID) ([]database.User, error) {
-	if err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceSystem); err != nil {
-		return nil, err
-	}
-	return q.db.GetUsersByIDs(ctx, ids)
-}
-
 func (q *querier) GetProvisionerJobsByIDs(ctx context.Context, ids []uuid.UUID) ([]database.ProvisionerJob, error) {
 	if err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceSystem); err != nil {
 		return nil, err
diff --git a/coderd/database/dbauthz/system_test.go b/coderd/database/dbauthz/system_test.go
@@ -157,13 +157,6 @@ func (s *MethodTestSuite) TestSystemFunctions() {
 			Asserts(rbac.ResourceSystem, rbac.ActionRead).
 			Returns(slice.New(tv1, tv2, tv3))
 	}))
-	s.Run("GetUsersByIDs", s.Subtest(func(db database.Store, check *expects) {
-		a := dbgen.User(s.T(), db, database.User{CreatedAt: database.Now().Add(-time.Hour)})
-		b := dbgen.User(s.T(), db, database.User{CreatedAt: database.Now()})
-		check.Args([]uuid.UUID{a.ID, b.ID}).
-			Asserts(rbac.ResourceSystem, rbac.ActionRead).
-			Returns(slice.New(a, b))
-	}))
 	s.Run("GetWorkspaceAppsByAgentIDs", s.Subtest(func(db database.Store, check *expects) {
 		aWs := dbgen.Workspace(s.T(), db, database.Workspace{})
 		aBuild := dbgen.WorkspaceBuild(s.T(), db, database.WorkspaceBuild{WorkspaceID: aWs.ID, JobID: uuid.New()})
