Skip to content

Commit ddabe9c

Browse files
mtojekmtojek
authored
feat: improve RBAC preconditions for Insights endpoint (#8794)
1 parent 4cc270b commit ddabe9c

File tree

4 files changed

+332
-16
lines changed

4 files changed

+332
-16
lines changed

‎coderd/database/dbauthz/dbauthz.go

Lines changed: 42 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1195,17 +1195,39 @@ func (q *querier) GetTemplateDAUs(ctx context.Context, arg database.GetTemplateD
11951195
}
11961196

11971197
func (q *querier) GetTemplateDailyInsights(ctx context.Context, arg database.GetTemplateDailyInsightsParams) ([]database.GetTemplateDailyInsightsRow, error) {
1198-
// FIXME: this should maybe be READ rbac.ResourceTemplate or it's own resource.
1199-
if err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceSystem); err != nil {
1200-
return nil, err
1198+
for _, templateID := range arg.TemplateIDs {
1199+
template, err := q.db.GetTemplateByID(ctx, templateID)
1200+
if err != nil {
1201+
return nil, err
1202+
}
1203+
1204+
if err := q.authorizeContext(ctx, rbac.ActionUpdate, template); err != nil {
1205+
return nil, err
1206+
}
1207+
}
1208+
if len(arg.TemplateIDs) == 0 {
1209+
if err := q.authorizeContext(ctx, rbac.ActionUpdate, rbac.ResourceTemplate.All()); err != nil {
1210+
return nil, err
1211+
}
12011212
}
12021213
return q.db.GetTemplateDailyInsights(ctx, arg)
12031214
}
12041215

12051216
func (q *querier) GetTemplateInsights(ctx context.Context, arg database.GetTemplateInsightsParams) (database.GetTemplateInsightsRow, error) {
1206-
// FIXME: this should maybe be READ rbac.ResourceTemplate or it's own resource.
1207-
if err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceSystem); err != nil {
1208-
return database.GetTemplateInsightsRow{}, err
1217+
for _, templateID := range arg.TemplateIDs {
1218+
template, err := q.db.GetTemplateByID(ctx, templateID)
1219+
if err != nil {
1220+
return database.GetTemplateInsightsRow{}, err
1221+
}
1222+
1223+
if err := q.authorizeContext(ctx, rbac.ActionUpdate, template); err != nil {
1224+
return database.GetTemplateInsightsRow{}, err
1225+
}
1226+
}
1227+
if len(arg.TemplateIDs) == 0 {
1228+
if err := q.authorizeContext(ctx, rbac.ActionUpdate, rbac.ResourceTemplate.All()); err != nil {
1229+
return database.GetTemplateInsightsRow{}, err
1230+
}
12091231
}
12101232
return q.db.GetTemplateInsights(ctx, arg)
12111233
}
@@ -1377,8 +1399,20 @@ func (q *querier) GetUserCount(ctx context.Context) (int64, error) {
13771399
}
13781400

13791401
func (q *querier) GetUserLatencyInsights(ctx context.Context, arg database.GetUserLatencyInsightsParams) ([]database.GetUserLatencyInsightsRow, error) {
1380-
if err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceSystem); err != nil {
1381-
return nil, err
1402+
for _, templateID := range arg.TemplateIDs {
1403+
template, err := q.db.GetTemplateByID(ctx, templateID)
1404+
if err != nil {
1405+
return nil, err
1406+
}
1407+
1408+
if err := q.authorizeContext(ctx, rbac.ActionUpdate, template); err != nil {
1409+
return nil, err
1410+
}
1411+
}
1412+
if len(arg.TemplateIDs) == 0 {
1413+
if err := q.authorizeContext(ctx, rbac.ActionUpdate, rbac.ResourceTemplate.All()); err != nil {
1414+
return nil, err
1415+
}
13821416
}
13831417
return q.db.GetUserLatencyInsights(ctx, arg)
13841418
}

‎coderd/insights.go

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -64,10 +64,6 @@ func (api *API) deploymentDAUs(rw http.ResponseWriter, r *http.Request) {
6464
// @Router /insights/user-latency [get]
6565
func (api *API) insightsUserLatency(rw http.ResponseWriter, r *http.Request) {
6666
ctx := r.Context()
67-
if !api.Authorize(r, rbac.ActionRead, rbac.ResourceDeploymentValues) {
68-
httpapi.Forbidden(rw)
69-
return
70-
}
7167

7268
p := httpapi.NewQueryParamParser().
7369
Required("start_time").
@@ -100,6 +96,10 @@ func (api *API) insightsUserLatency(rw http.ResponseWriter, r *http.Request) {
10096
TemplateIDs: templateIDs,
10197
})
10298
if err != nil {
99+
if httpapi.Is404Error(err) {
100+
httpapi.ResourceNotFound(rw)
101+
return
102+
}
103103
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
104104
Message: "Internal error fetching user latency.",
105105
Detail: err.Error(),
@@ -154,10 +154,6 @@ func (api *API) insightsUserLatency(rw http.ResponseWriter, r *http.Request) {
154154
// @Router /insights/templates [get]
155155
func (api *API) insightsTemplates(rw http.ResponseWriter, r *http.Request) {
156156
ctx := r.Context()
157-
if !api.Authorize(r, rbac.ActionRead, rbac.ResourceDeploymentValues) {
158-
httpapi.Forbidden(rw)
159-
return
160-
}
161157

162158
p := httpapi.NewQueryParamParser().
163159
Required("start_time").
@@ -191,6 +187,7 @@ func (api *API) insightsTemplates(rw http.ResponseWriter, r *http.Request) {
191187

192188
var usage database.GetTemplateInsightsRow
193189
var dailyUsage []database.GetTemplateDailyInsightsRow
190+
194191
// Use a transaction to ensure that we get consistent data between
195192
// the full and interval report.
196193
err := api.Database.InTx(func(db database.Store) error {
@@ -218,6 +215,10 @@ func (api *API) insightsTemplates(rw http.ResponseWriter, r *http.Request) {
218215

219216
return nil
220217
}, nil)
218+
if httpapi.Is404Error(err) {
219+
httpapi.ResourceNotFound(rw)
220+
return
221+
}
221222
if err != nil {
222223
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
223224
Message: "Internal error fetching template insights.",

‎coderd/insights_test.go

Lines changed: 213 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@ package coderd_test
22

33
import (
44
"context"
5+
"fmt"
56
"io"
7+
"net/http"
68
"testing"
79
"time"
810

@@ -14,6 +16,7 @@ import (
1416
"cdr.dev/slog/sloggers/slogtest"
1517
"github.com/coder/coder/agent"
1618
"github.com/coder/coder/coderd/coderdtest"
19+
"github.com/coder/coder/coderd/rbac"
1720
"github.com/coder/coder/codersdk"
1821
"github.com/coder/coder/codersdk/agentsdk"
1922
"github.com/coder/coder/provisioner/echo"
@@ -380,3 +383,213 @@ func TestTemplateInsights_BadRequest(t *testing.T) {
380383
})
381384
assert.Error(t, err, "want error for bad interval")
382385
}
386+
387+
func TestTemplateInsights_RBAC(t *testing.T) {
388+
t.Parallel()
389+
390+
y, m, d := time.Now().UTC().Date()
391+
today := time.Date(y, m, d, 0, 0, 0, 0, time.UTC)
392+
393+
type test struct {
394+
interval codersdk.InsightsReportInterval
395+
withTemplate bool
396+
}
397+
398+
tests := []test{
399+
{codersdk.InsightsReportIntervalDay, true},
400+
{codersdk.InsightsReportIntervalDay, false},
401+
{"", true},
402+
{"", false},
403+
}
404+
405+
for _, tt := range tests {
406+
tt := tt
407+
408+
t.Run(fmt.Sprintf("with interval=%q", tt.interval), func(t *testing.T) {
409+
t.Parallel()
410+
411+
t.Run("AsOwner", func(t *testing.T) {
412+
t.Parallel()
413+
414+
client := coderdtest.New(t, &coderdtest.Options{})
415+
admin := coderdtest.CreateFirstUser(t, client)
416+
417+
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort)
418+
defer cancel()
419+
420+
var templateIDs []uuid.UUID
421+
if tt.withTemplate {
422+
version := coderdtest.CreateTemplateVersion(t, client, admin.OrganizationID, nil)
423+
template := coderdtest.CreateTemplate(t, client, admin.OrganizationID, version.ID)
424+
templateIDs = append(templateIDs, template.ID)
425+
}
426+
427+
_, err := client.TemplateInsights(ctx, codersdk.TemplateInsightsRequest{
428+
StartTime: today.AddDate(0, 0, -1),
429+
EndTime: today,
430+
Interval: tt.interval,
431+
TemplateIDs: templateIDs,
432+
})
433+
require.NoError(t, err)
434+
})
435+
t.Run("AsTemplateAdmin", func(t *testing.T) {
436+
t.Parallel()
437+
438+
client := coderdtest.New(t, &coderdtest.Options{})
439+
admin := coderdtest.CreateFirstUser(t, client)
440+
441+
templateAdmin, _ := coderdtest.CreateAnotherUser(t, client, admin.OrganizationID, rbac.RoleTemplateAdmin())
442+
443+
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort)
444+
defer cancel()
445+
446+
var templateIDs []uuid.UUID
447+
if tt.withTemplate {
448+
version := coderdtest.CreateTemplateVersion(t, client, admin.OrganizationID, nil)
449+
template := coderdtest.CreateTemplate(t, client, admin.OrganizationID, version.ID)
450+
templateIDs = append(templateIDs, template.ID)
451+
}
452+
453+
_, err := templateAdmin.TemplateInsights(ctx, codersdk.TemplateInsightsRequest{
454+
StartTime: today.AddDate(0, 0, -1),
455+
EndTime: today,
456+
Interval: tt.interval,
457+
TemplateIDs: templateIDs,
458+
})
459+
require.NoError(t, err)
460+
})
461+
t.Run("AsRegularUser", func(t *testing.T) {
462+
t.Parallel()
463+
464+
client := coderdtest.New(t, &coderdtest.Options{})
465+
admin := coderdtest.CreateFirstUser(t, client)
466+
467+
regular, _ := coderdtest.CreateAnotherUser(t, client, admin.OrganizationID)
468+
469+
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort)
470+
defer cancel()
471+
472+
var templateIDs []uuid.UUID
473+
if tt.withTemplate {
474+
version := coderdtest.CreateTemplateVersion(t, client, admin.OrganizationID, nil)
475+
template := coderdtest.CreateTemplate(t, client, admin.OrganizationID, version.ID)
476+
templateIDs = append(templateIDs, template.ID)
477+
}
478+
479+
_, err := regular.TemplateInsights(ctx, codersdk.TemplateInsightsRequest{
480+
StartTime: today.AddDate(0, 0, -1),
481+
EndTime: today,
482+
Interval: tt.interval,
483+
TemplateIDs: templateIDs,
484+
})
485+
require.Error(t, err)
486+
var apiErr *codersdk.Error
487+
require.ErrorAs(t, err, &apiErr)
488+
require.Equal(t, http.StatusNotFound, apiErr.StatusCode())
489+
})
490+
})
491+
}
492+
}
493+
494+
func TestUserLatencyInsights_RBAC(t *testing.T) {
495+
t.Parallel()
496+
497+
y, m, d := time.Now().UTC().Date()
498+
today := time.Date(y, m, d, 0, 0, 0, 0, time.UTC)
499+
500+
type test struct {
501+
interval codersdk.InsightsReportInterval
502+
withTemplate bool
503+
}
504+
505+
tests := []test{
506+
{codersdk.InsightsReportIntervalDay, true},
507+
{codersdk.InsightsReportIntervalDay, false},
508+
{"", true},
509+
{"", false},
510+
}
511+
512+
for _, tt := range tests {
513+
tt := tt
514+
t.Run(fmt.Sprintf("with interval=%q", tt.interval), func(t *testing.T) {
515+
t.Parallel()
516+
517+
t.Run("AsOwner", func(t *testing.T) {
518+
t.Parallel()
519+
520+
client := coderdtest.New(t, &coderdtest.Options{})
521+
admin := coderdtest.CreateFirstUser(t, client)
522+
523+
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort)
524+
defer cancel()
525+
526+
var templateIDs []uuid.UUID
527+
if tt.withTemplate {
528+
version := coderdtest.CreateTemplateVersion(t, client, admin.OrganizationID, nil)
529+
template := coderdtest.CreateTemplate(t, client, admin.OrganizationID, version.ID)
530+
templateIDs = append(templateIDs, template.ID)
531+
}
532+
533+
_, err := client.UserLatencyInsights(ctx, codersdk.UserLatencyInsightsRequest{
534+
StartTime: today,
535+
EndTime: time.Now().UTC().Truncate(time.Hour).Add(time.Hour), // Round up to include the current hour.
536+
TemplateIDs: templateIDs,
537+
})
538+
require.NoError(t, err)
539+
})
540+
t.Run("AsTemplateAdmin", func(t *testing.T) {
541+
t.Parallel()
542+
543+
client := coderdtest.New(t, &coderdtest.Options{})
544+
admin := coderdtest.CreateFirstUser(t, client)
545+
546+
templateAdmin, _ := coderdtest.CreateAnotherUser(t, client, admin.OrganizationID, rbac.RoleTemplateAdmin())
547+
548+
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort)
549+
defer cancel()
550+
551+
var templateIDs []uuid.UUID
552+
if tt.withTemplate {
553+
version := coderdtest.CreateTemplateVersion(t, client, admin.OrganizationID, nil)
554+
template := coderdtest.CreateTemplate(t, client, admin.OrganizationID, version.ID)
555+
templateIDs = append(templateIDs, template.ID)
556+
}
557+
558+
_, err := templateAdmin.UserLatencyInsights(ctx, codersdk.UserLatencyInsightsRequest{
559+
StartTime: today,
560+
EndTime: time.Now().UTC().Truncate(time.Hour).Add(time.Hour), // Round up to include the current hour.
561+
TemplateIDs: templateIDs,
562+
})
563+
require.NoError(t, err)
564+
})
565+
t.Run("AsRegularUser", func(t *testing.T) {
566+
t.Parallel()
567+
568+
client := coderdtest.New(t, &coderdtest.Options{})
569+
admin := coderdtest.CreateFirstUser(t, client)
570+
571+
regular, _ := coderdtest.CreateAnotherUser(t, client, admin.OrganizationID)
572+
573+
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort)
574+
defer cancel()
575+
576+
var templateIDs []uuid.UUID
577+
if tt.withTemplate {
578+
version := coderdtest.CreateTemplateVersion(t, client, admin.OrganizationID, nil)
579+
template := coderdtest.CreateTemplate(t, client, admin.OrganizationID, version.ID)
580+
templateIDs = append(templateIDs, template.ID)
581+
}
582+
583+
_, err := regular.UserLatencyInsights(ctx, codersdk.UserLatencyInsightsRequest{
584+
StartTime: today,
585+
EndTime: time.Now().UTC().Truncate(time.Hour).Add(time.Hour), // Round up to include the current hour.
586+
TemplateIDs: templateIDs,
587+
})
588+
require.Error(t, err)
589+
var apiErr *codersdk.Error
590+
require.ErrorAs(t, err, &apiErr)
591+
require.Equal(t, http.StatusNotFound, apiErr.StatusCode())
592+
})
593+
})
594+
}
595+
}

0 commit comments

Comments
 (0)