diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 9bc3a187f6d84..f3ea758dce9f9 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -1195,17 +1195,39 @@ func (q *querier) GetTemplateDAUs(ctx context.Context, arg database.GetTemplateD } func (q *querier) GetTemplateDailyInsights(ctx context.Context, arg database.GetTemplateDailyInsightsParams) ([]database.GetTemplateDailyInsightsRow, error) { - // FIXME: this should maybe be READ rbac.ResourceTemplate or it's own resource. - if err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceSystem); err != nil { - return nil, err + for _, templateID := range arg.TemplateIDs { + template, err := q.db.GetTemplateByID(ctx, templateID) + if err != nil { + return nil, err + } + + if err := q.authorizeContext(ctx, rbac.ActionUpdate, template); err != nil { + return nil, err + } + } + if len(arg.TemplateIDs) == 0 { + if err := q.authorizeContext(ctx, rbac.ActionUpdate, rbac.ResourceTemplate.All()); err != nil { + return nil, err + } } return q.db.GetTemplateDailyInsights(ctx, arg) } func (q *querier) GetTemplateInsights(ctx context.Context, arg database.GetTemplateInsightsParams) (database.GetTemplateInsightsRow, error) { - // FIXME: this should maybe be READ rbac.ResourceTemplate or it's own resource. - if err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceSystem); err != nil { - return database.GetTemplateInsightsRow{}, err + for _, templateID := range arg.TemplateIDs { + template, err := q.db.GetTemplateByID(ctx, templateID) + if err != nil { + return database.GetTemplateInsightsRow{}, err + } + + if err := q.authorizeContext(ctx, rbac.ActionUpdate, template); err != nil { + return database.GetTemplateInsightsRow{}, err + } + } + if len(arg.TemplateIDs) == 0 { + if err := q.authorizeContext(ctx, rbac.ActionUpdate, rbac.ResourceTemplate.All()); err != nil { + return database.GetTemplateInsightsRow{}, err + } } return q.db.GetTemplateInsights(ctx, arg) } @@ -1377,8 +1399,20 @@ func (q *querier) GetUserCount(ctx context.Context) (int64, error) { } func (q *querier) GetUserLatencyInsights(ctx context.Context, arg database.GetUserLatencyInsightsParams) ([]database.GetUserLatencyInsightsRow, error) { - if err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceSystem); err != nil { - return nil, err + for _, templateID := range arg.TemplateIDs { + template, err := q.db.GetTemplateByID(ctx, templateID) + if err != nil { + return nil, err + } + + if err := q.authorizeContext(ctx, rbac.ActionUpdate, template); err != nil { + return nil, err + } + } + if len(arg.TemplateIDs) == 0 { + if err := q.authorizeContext(ctx, rbac.ActionUpdate, rbac.ResourceTemplate.All()); err != nil { + return nil, err + } } return q.db.GetUserLatencyInsights(ctx, arg) } diff --git a/coderd/insights.go b/coderd/insights.go index 2c6d22be0ab53..9152f14983814 100644 --- a/coderd/insights.go +++ b/coderd/insights.go @@ -64,10 +64,6 @@ func (api *API) deploymentDAUs(rw http.ResponseWriter, r *http.Request) { // @Router /insights/user-latency [get] func (api *API) insightsUserLatency(rw http.ResponseWriter, r *http.Request) { ctx := r.Context() - if !api.Authorize(r, rbac.ActionRead, rbac.ResourceDeploymentValues) { - httpapi.Forbidden(rw) - return - } p := httpapi.NewQueryParamParser(). Required("start_time"). @@ -100,6 +96,10 @@ func (api *API) insightsUserLatency(rw http.ResponseWriter, r *http.Request) { TemplateIDs: templateIDs, }) if err != nil { + if httpapi.Is404Error(err) { + httpapi.ResourceNotFound(rw) + return + } httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ Message: "Internal error fetching user latency.", Detail: err.Error(), @@ -154,10 +154,6 @@ func (api *API) insightsUserLatency(rw http.ResponseWriter, r *http.Request) { // @Router /insights/templates [get] func (api *API) insightsTemplates(rw http.ResponseWriter, r *http.Request) { ctx := r.Context() - if !api.Authorize(r, rbac.ActionRead, rbac.ResourceDeploymentValues) { - httpapi.Forbidden(rw) - return - } p := httpapi.NewQueryParamParser(). Required("start_time"). @@ -191,6 +187,7 @@ func (api *API) insightsTemplates(rw http.ResponseWriter, r *http.Request) { var usage database.GetTemplateInsightsRow var dailyUsage []database.GetTemplateDailyInsightsRow + // Use a transaction to ensure that we get consistent data between // the full and interval report. err := api.Database.InTx(func(db database.Store) error { @@ -218,6 +215,10 @@ func (api *API) insightsTemplates(rw http.ResponseWriter, r *http.Request) { return nil }, nil) + if httpapi.Is404Error(err) { + httpapi.ResourceNotFound(rw) + return + } if err != nil { httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ Message: "Internal error fetching template insights.", diff --git a/coderd/insights_test.go b/coderd/insights_test.go index f108882299b9a..8f518e0746076 100644 --- a/coderd/insights_test.go +++ b/coderd/insights_test.go @@ -2,7 +2,9 @@ package coderd_test import ( "context" + "fmt" "io" + "net/http" "testing" "time" @@ -14,6 +16,7 @@ import ( "cdr.dev/slog/sloggers/slogtest" "github.com/coder/coder/agent" "github.com/coder/coder/coderd/coderdtest" + "github.com/coder/coder/coderd/rbac" "github.com/coder/coder/codersdk" "github.com/coder/coder/codersdk/agentsdk" "github.com/coder/coder/provisioner/echo" @@ -380,3 +383,213 @@ func TestTemplateInsights_BadRequest(t *testing.T) { }) assert.Error(t, err, "want error for bad interval") } + +func TestTemplateInsights_RBAC(t *testing.T) { + t.Parallel() + + y, m, d := time.Now().UTC().Date() + today := time.Date(y, m, d, 0, 0, 0, 0, time.UTC) + + type test struct { + interval codersdk.InsightsReportInterval + withTemplate bool + } + + tests := []test{ + {codersdk.InsightsReportIntervalDay, true}, + {codersdk.InsightsReportIntervalDay, false}, + {"", true}, + {"", false}, + } + + for _, tt := range tests { + tt := tt + + t.Run(fmt.Sprintf("with interval=%q", tt.interval), func(t *testing.T) { + t.Parallel() + + t.Run("AsOwner", func(t *testing.T) { + t.Parallel() + + client := coderdtest.New(t, &coderdtest.Options{}) + admin := coderdtest.CreateFirstUser(t, client) + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort) + defer cancel() + + var templateIDs []uuid.UUID + if tt.withTemplate { + version := coderdtest.CreateTemplateVersion(t, client, admin.OrganizationID, nil) + template := coderdtest.CreateTemplate(t, client, admin.OrganizationID, version.ID) + templateIDs = append(templateIDs, template.ID) + } + + _, err := client.TemplateInsights(ctx, codersdk.TemplateInsightsRequest{ + StartTime: today.AddDate(0, 0, -1), + EndTime: today, + Interval: tt.interval, + TemplateIDs: templateIDs, + }) + require.NoError(t, err) + }) + t.Run("AsTemplateAdmin", func(t *testing.T) { + t.Parallel() + + client := coderdtest.New(t, &coderdtest.Options{}) + admin := coderdtest.CreateFirstUser(t, client) + + templateAdmin, _ := coderdtest.CreateAnotherUser(t, client, admin.OrganizationID, rbac.RoleTemplateAdmin()) + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort) + defer cancel() + + var templateIDs []uuid.UUID + if tt.withTemplate { + version := coderdtest.CreateTemplateVersion(t, client, admin.OrganizationID, nil) + template := coderdtest.CreateTemplate(t, client, admin.OrganizationID, version.ID) + templateIDs = append(templateIDs, template.ID) + } + + _, err := templateAdmin.TemplateInsights(ctx, codersdk.TemplateInsightsRequest{ + StartTime: today.AddDate(0, 0, -1), + EndTime: today, + Interval: tt.interval, + TemplateIDs: templateIDs, + }) + require.NoError(t, err) + }) + t.Run("AsRegularUser", func(t *testing.T) { + t.Parallel() + + client := coderdtest.New(t, &coderdtest.Options{}) + admin := coderdtest.CreateFirstUser(t, client) + + regular, _ := coderdtest.CreateAnotherUser(t, client, admin.OrganizationID) + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort) + defer cancel() + + var templateIDs []uuid.UUID + if tt.withTemplate { + version := coderdtest.CreateTemplateVersion(t, client, admin.OrganizationID, nil) + template := coderdtest.CreateTemplate(t, client, admin.OrganizationID, version.ID) + templateIDs = append(templateIDs, template.ID) + } + + _, err := regular.TemplateInsights(ctx, codersdk.TemplateInsightsRequest{ + StartTime: today.AddDate(0, 0, -1), + EndTime: today, + Interval: tt.interval, + TemplateIDs: templateIDs, + }) + require.Error(t, err) + var apiErr *codersdk.Error + require.ErrorAs(t, err, &apiErr) + require.Equal(t, http.StatusNotFound, apiErr.StatusCode()) + }) + }) + } +} + +func TestUserLatencyInsights_RBAC(t *testing.T) { + t.Parallel() + + y, m, d := time.Now().UTC().Date() + today := time.Date(y, m, d, 0, 0, 0, 0, time.UTC) + + type test struct { + interval codersdk.InsightsReportInterval + withTemplate bool + } + + tests := []test{ + {codersdk.InsightsReportIntervalDay, true}, + {codersdk.InsightsReportIntervalDay, false}, + {"", true}, + {"", false}, + } + + for _, tt := range tests { + tt := tt + t.Run(fmt.Sprintf("with interval=%q", tt.interval), func(t *testing.T) { + t.Parallel() + + t.Run("AsOwner", func(t *testing.T) { + t.Parallel() + + client := coderdtest.New(t, &coderdtest.Options{}) + admin := coderdtest.CreateFirstUser(t, client) + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort) + defer cancel() + + var templateIDs []uuid.UUID + if tt.withTemplate { + version := coderdtest.CreateTemplateVersion(t, client, admin.OrganizationID, nil) + template := coderdtest.CreateTemplate(t, client, admin.OrganizationID, version.ID) + templateIDs = append(templateIDs, template.ID) + } + + _, err := client.UserLatencyInsights(ctx, codersdk.UserLatencyInsightsRequest{ + StartTime: today, + EndTime: time.Now().UTC().Truncate(time.Hour).Add(time.Hour), // Round up to include the current hour. + TemplateIDs: templateIDs, + }) + require.NoError(t, err) + }) + t.Run("AsTemplateAdmin", func(t *testing.T) { + t.Parallel() + + client := coderdtest.New(t, &coderdtest.Options{}) + admin := coderdtest.CreateFirstUser(t, client) + + templateAdmin, _ := coderdtest.CreateAnotherUser(t, client, admin.OrganizationID, rbac.RoleTemplateAdmin()) + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort) + defer cancel() + + var templateIDs []uuid.UUID + if tt.withTemplate { + version := coderdtest.CreateTemplateVersion(t, client, admin.OrganizationID, nil) + template := coderdtest.CreateTemplate(t, client, admin.OrganizationID, version.ID) + templateIDs = append(templateIDs, template.ID) + } + + _, err := templateAdmin.UserLatencyInsights(ctx, codersdk.UserLatencyInsightsRequest{ + StartTime: today, + EndTime: time.Now().UTC().Truncate(time.Hour).Add(time.Hour), // Round up to include the current hour. + TemplateIDs: templateIDs, + }) + require.NoError(t, err) + }) + t.Run("AsRegularUser", func(t *testing.T) { + t.Parallel() + + client := coderdtest.New(t, &coderdtest.Options{}) + admin := coderdtest.CreateFirstUser(t, client) + + regular, _ := coderdtest.CreateAnotherUser(t, client, admin.OrganizationID) + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort) + defer cancel() + + var templateIDs []uuid.UUID + if tt.withTemplate { + version := coderdtest.CreateTemplateVersion(t, client, admin.OrganizationID, nil) + template := coderdtest.CreateTemplate(t, client, admin.OrganizationID, version.ID) + templateIDs = append(templateIDs, template.ID) + } + + _, err := regular.UserLatencyInsights(ctx, codersdk.UserLatencyInsightsRequest{ + StartTime: today, + EndTime: time.Now().UTC().Truncate(time.Hour).Add(time.Hour), // Round up to include the current hour. + TemplateIDs: templateIDs, + }) + require.Error(t, err) + var apiErr *codersdk.Error + require.ErrorAs(t, err, &apiErr) + require.Equal(t, http.StatusNotFound, apiErr.StatusCode()) + }) + }) + } +} diff --git a/enterprise/coderd/insights_test.go b/enterprise/coderd/insights_test.go new file mode 100644 index 0000000000000..be74c8dad2808 --- /dev/null +++ b/enterprise/coderd/insights_test.go @@ -0,0 +1,68 @@ +package coderd_test + +import ( + "context" + "fmt" + "testing" + "time" + + "github.com/google/uuid" + "github.com/stretchr/testify/require" + + "github.com/coder/coder/coderd/coderdtest" + "github.com/coder/coder/codersdk" + "github.com/coder/coder/enterprise/coderd/coderdenttest" + "github.com/coder/coder/enterprise/coderd/license" + "github.com/coder/coder/testutil" +) + +func TestTemplateInsightsWithTemplateAdminACL(t *testing.T) { + t.Parallel() + + y, m, d := time.Now().UTC().Date() + today := time.Date(y, m, d, 0, 0, 0, 0, time.UTC) + + type test struct { + interval codersdk.InsightsReportInterval + } + + tests := []test{ + {codersdk.InsightsReportIntervalDay}, + {""}, + } + + for _, tt := range tests { + tt := tt + t.Run(fmt.Sprintf("with interval=%q", tt.interval), func(t *testing.T) { + t.Parallel() + + client, admin := coderdenttest.New(t, &coderdenttest.Options{LicenseOptions: &coderdenttest.LicenseOptions{ + Features: license.Features{ + codersdk.FeatureTemplateRBAC: 1, + }, + }}) + + version := coderdtest.CreateTemplateVersion(t, client, admin.OrganizationID, nil) + template := coderdtest.CreateTemplate(t, client, admin.OrganizationID, version.ID) + + regular, regularUser := coderdtest.CreateAnotherUser(t, client, admin.OrganizationID) + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort) + defer cancel() + + err := client.UpdateTemplateACL(ctx, template.ID, codersdk.UpdateTemplateACL{ + UserPerms: map[string]codersdk.TemplateRole{ + regularUser.ID.String(): codersdk.TemplateRoleAdmin, + }, + }) + require.NoError(t, err) + + _, err = regular.TemplateInsights(ctx, codersdk.TemplateInsightsRequest{ + StartTime: today.AddDate(0, 0, -1), + EndTime: today, + TemplateIDs: []uuid.UUID{template.ID}, + }) + require.NoError(t, err) + }) + } +}