Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Commit30e6fbd

Browse files
authored
fix(coderd): ensure correct RBAC when enqueueing notifications (#15478)
- Assert rbac in fake notifications enqueuer- Move fake notifications enqueuer to separate notificationstest package- Update dbauthz rbac policy to allow provisionerd and autostart to create and read notification messages- Update tests as required
1 parentbb5c3a2 commit30e6fbd

File tree

18 files changed

+323
-242
lines changed

18 files changed

+323
-242
lines changed

‎coderd/autobuild/lifecycle_executor_test.go‎

Lines changed: 24 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
"github.com/coder/coder/v2/coderd/database"
2020
"github.com/coder/coder/v2/coderd/database/dbauthz"
2121
"github.com/coder/coder/v2/coderd/notifications"
22+
"github.com/coder/coder/v2/coderd/notifications/notificationstest"
2223
"github.com/coder/coder/v2/coderd/schedule"
2324
"github.com/coder/coder/v2/coderd/schedule/cron"
2425
"github.com/coder/coder/v2/coderd/util/ptr"
@@ -116,7 +117,7 @@ func TestExecutorAutostartTemplateUpdated(t *testing.T) {
116117
tickCh=make(chan time.Time)
117118
statsCh=make(chan autobuild.Stats)
118119
logger=slogtest.Make(t,&slogtest.Options{IgnoreErrors:!tc.expectStart}).Leveled(slog.LevelDebug)
119-
enqueuer=testutil.FakeNotificationsEnqueuer{}
120+
enqueuer=notificationstest.FakeEnqueuer{}
120121
client=coderdtest.New(t,&coderdtest.Options{
121122
AutobuildTicker:tickCh,
122123
IncludeProvisionerDaemon:true,
@@ -202,17 +203,18 @@ func TestExecutorAutostartTemplateUpdated(t *testing.T) {
202203
}
203204

204205
iftc.expectNotification {
205-
require.Len(t,enqueuer.Sent,1)
206-
require.Equal(t,enqueuer.Sent[0].UserID,workspace.OwnerID)
207-
require.Contains(t,enqueuer.Sent[0].Targets,workspace.TemplateID)
208-
require.Contains(t,enqueuer.Sent[0].Targets,workspace.ID)
209-
require.Contains(t,enqueuer.Sent[0].Targets,workspace.OrganizationID)
210-
require.Contains(t,enqueuer.Sent[0].Targets,workspace.OwnerID)
211-
require.Equal(t,newVersion.Name,enqueuer.Sent[0].Labels["template_version_name"])
212-
require.Equal(t,"autobuild",enqueuer.Sent[0].Labels["initiator"])
213-
require.Equal(t,"autostart",enqueuer.Sent[0].Labels["reason"])
206+
sent:=enqueuer.Sent()
207+
require.Len(t,sent,1)
208+
require.Equal(t,sent[0].UserID,workspace.OwnerID)
209+
require.Contains(t,sent[0].Targets,workspace.TemplateID)
210+
require.Contains(t,sent[0].Targets,workspace.ID)
211+
require.Contains(t,sent[0].Targets,workspace.OrganizationID)
212+
require.Contains(t,sent[0].Targets,workspace.OwnerID)
213+
require.Equal(t,newVersion.Name,sent[0].Labels["template_version_name"])
214+
require.Equal(t,"autobuild",sent[0].Labels["initiator"])
215+
require.Equal(t,"autostart",sent[0].Labels["reason"])
214216
}else {
215-
require.Len(t,enqueuer.Sent,0)
217+
require.Empty(t,enqueuer.Sent())
216218
}
217219
})
218220
}
@@ -1073,7 +1075,7 @@ func TestNotifications(t *testing.T) {
10731075
var (
10741076
ticker=make(chan time.Time)
10751077
statCh=make(chan autobuild.Stats)
1076-
notifyEnq=testutil.FakeNotificationsEnqueuer{}
1078+
notifyEnq=notificationstest.FakeEnqueuer{}
10771079
timeTilDormant=time.Minute
10781080
client=coderdtest.New(t,&coderdtest.Options{
10791081
AutobuildTicker:ticker,
@@ -1107,6 +1109,7 @@ func TestNotifications(t *testing.T) {
11071109
_=coderdtest.AwaitWorkspaceBuildJobCompleted(t,userClient,workspace.LatestBuild.ID)
11081110

11091111
// Wait for workspace to become dormant
1112+
notifyEnq.Clear()
11101113
ticker<-workspace.LastUsedAt.Add(timeTilDormant*3)
11111114
_=testutil.RequireRecvCtx(testutil.Context(t,testutil.WaitShort),t,statCh)
11121115

@@ -1115,14 +1118,14 @@ func TestNotifications(t *testing.T) {
11151118
require.NotNil(t,workspace.DormantAt)
11161119

11171120
// Check that a notification was enqueued
1118-
require.Len(t,notifyEnq.Sent,2)
1119-
// notifyEnq.Sent[0] is an event for created user account
1120-
require.Equal(t,notifyEnq.Sent[1].UserID,workspace.OwnerID)
1121-
require.Equal(t,notifyEnq.Sent[1].TemplateID,notifications.TemplateWorkspaceDormant)
1122-
require.Contains(t,notifyEnq.Sent[1].Targets,template.ID)
1123-
require.Contains(t,notifyEnq.Sent[1].Targets,workspace.ID)
1124-
require.Contains(t,notifyEnq.Sent[1].Targets,workspace.OrganizationID)
1125-
require.Contains(t,notifyEnq.Sent[1].Targets,workspace.OwnerID)
1121+
sent:=notifyEnq.Sent()
1122+
require.Len(t,sent,1)
1123+
require.Equal(t,sent[0].UserID,workspace.OwnerID)
1124+
require.Equal(t,sent[0].TemplateID,notifications.TemplateWorkspaceDormant)
1125+
require.Contains(t,sent[0].Targets,template.ID)
1126+
require.Contains(t,sent[0].Targets,workspace.ID)
1127+
require.Contains(t,sent[0].Targets,workspace.OrganizationID)
1128+
require.Contains(t,sent[0].Targets,workspace.OwnerID)
11261129
})
11271130
}
11281131

@@ -1168,7 +1171,7 @@ func mustSchedule(t *testing.T, s string) *cron.Schedule {
11681171
}
11691172

11701173
funcmustWorkspaceParameters(t*testing.T,client*codersdk.Client,workspaceID uuid.UUID) {
1171-
ctx:=context.Background()
1174+
ctx:=testutil.Context(t,testutil.WaitShort)
11721175
buildParameters,err:=client.WorkspaceBuildParameters(ctx,workspaceID)
11731176
require.NoError(t,err)
11741177
require.NotEmpty(t,buildParameters)

‎coderd/coderdtest/coderdtest.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ import (
6666
"github.com/coder/coder/v2/coderd/gitsshkey"
6767
"github.com/coder/coder/v2/coderd/httpmw"
6868
"github.com/coder/coder/v2/coderd/notifications"
69+
"github.com/coder/coder/v2/coderd/notifications/notificationstest"
6970
"github.com/coder/coder/v2/coderd/rbac"
7071
"github.com/coder/coder/v2/coderd/rbac/policy"
7172
"github.com/coder/coder/v2/coderd/runtimeconfig"
@@ -251,7 +252,7 @@ func NewOptions(t testing.TB, options *Options) (func(http.Handler), context.Can
251252
}
252253

253254
ifoptions.NotificationsEnqueuer==nil {
254-
options.NotificationsEnqueuer=new(testutil.FakeNotificationsEnqueuer)
255+
options.NotificationsEnqueuer=&notificationstest.FakeEnqueuer{}
255256
}
256257

257258
accessControlStore:=&atomic.Pointer[dbauthz.AccessControlStore]{}
@@ -311,7 +312,7 @@ func NewOptions(t testing.TB, options *Options) (func(http.Handler), context.Can
311312
t.Cleanup(closeBatcher)
312313
}
313314
ifoptions.NotificationsEnqueuer==nil {
314-
options.NotificationsEnqueuer=&testutil.FakeNotificationsEnqueuer{}
315+
options.NotificationsEnqueuer=&notificationstest.FakeEnqueuer{}
315316
}
316317

317318
ifoptions.OneTimePasscodeValidityPeriod==0 {

‎coderd/database/dbauthz/dbauthz.go‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,8 @@ var (
178178
// this can be reduced to read a specific org.
179179
rbac.ResourceOrganization.Type: {policy.ActionRead},
180180
rbac.ResourceGroup.Type: {policy.ActionRead},
181+
// Provisionerd creates notification messages
182+
rbac.ResourceNotificationMessage.Type: {policy.ActionCreate,policy.ActionRead},
181183
}),
182184
Org:map[string][]rbac.Permission{},
183185
User: []rbac.Permission{},
@@ -194,11 +196,12 @@ var (
194196
Identifier: rbac.RoleIdentifier{Name:"autostart"},
195197
DisplayName:"Autostart Daemon",
196198
Site:rbac.Permissions(map[string][]policy.Action{
197-
rbac.ResourceSystem.Type: {policy.WildcardSymbol},
198-
rbac.ResourceTemplate.Type: {policy.ActionRead,policy.ActionUpdate},
199-
rbac.ResourceWorkspaceDormant.Type: {policy.ActionDelete,policy.ActionRead,policy.ActionUpdate,policy.ActionWorkspaceStop},
200-
rbac.ResourceWorkspace.Type: {policy.ActionDelete,policy.ActionRead,policy.ActionUpdate,policy.ActionWorkspaceStart,policy.ActionWorkspaceStop},
201-
rbac.ResourceUser.Type: {policy.ActionRead},
199+
rbac.ResourceNotificationMessage.Type: {policy.ActionCreate,policy.ActionRead},
200+
rbac.ResourceSystem.Type: {policy.WildcardSymbol},
201+
rbac.ResourceTemplate.Type: {policy.ActionRead,policy.ActionUpdate},
202+
rbac.ResourceUser.Type: {policy.ActionRead},
203+
rbac.ResourceWorkspace.Type: {policy.ActionDelete,policy.ActionRead,policy.ActionUpdate,policy.ActionWorkspaceStart,policy.ActionWorkspaceStop},
204+
rbac.ResourceWorkspaceDormant.Type: {policy.ActionDelete,policy.ActionRead,policy.ActionUpdate,policy.ActionWorkspaceStop},
202205
}),
203206
Org:map[string][]rbac.Permission{},
204207
User: []rbac.Permission{},
Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
package notificationstest
2+
3+
import (
4+
"context"
5+
"fmt"
6+
"sync"
7+
8+
"github.com/google/uuid"
9+
"github.com/prometheus/client_golang/prometheus"
10+
11+
"github.com/coder/coder/v2/coderd/database/dbauthz"
12+
"github.com/coder/coder/v2/coderd/rbac"
13+
"github.com/coder/coder/v2/coderd/rbac/policy"
14+
)
15+
16+
typeFakeEnqueuerstruct {
17+
authorizer rbac.Authorizer
18+
mu sync.Mutex
19+
sent []*FakeNotification
20+
}
21+
22+
typeFakeNotificationstruct {
23+
UserID,TemplateID uuid.UUID
24+
Labelsmap[string]string
25+
Datamap[string]any
26+
CreatedBystring
27+
Targets []uuid.UUID
28+
}
29+
30+
// TODO: replace this with actual calls to dbauthz.
31+
// See: https://github.com/coder/coder/issues/15481
32+
func (f*FakeEnqueuer)assertRBACNoLock(ctx context.Context) {
33+
iff.mu.TryLock() {
34+
panic("Developer error: do not call assertRBACNoLock outside of a mutex lock!")
35+
}
36+
37+
// If we get here, we are locked.
38+
iff.authorizer==nil {
39+
f.authorizer=rbac.NewStrictCachingAuthorizer(prometheus.NewRegistry())
40+
}
41+
42+
act,ok:=dbauthz.ActorFromContext(ctx)
43+
if!ok {
44+
panic("Developer error: no actor in context, you may need to use dbauthz.AsNotifier(ctx)")
45+
}
46+
47+
for_,a:=range []policy.Action{policy.ActionCreate,policy.ActionRead} {
48+
err:=f.authorizer.Authorize(ctx,act,a,rbac.ResourceNotificationMessage)
49+
iferr==nil {
50+
return
51+
}
52+
53+
ifrbac.IsUnauthorizedError(err) {
54+
panic(fmt.Sprintf("Developer error: not authorized to %s %s. "+
55+
"Ensure that you are using dbauthz.AsXXX with an actor that has "+
56+
"policy.ActionCreate on rbac.ResourceNotificationMessage",a,rbac.ResourceNotificationMessage.Type))
57+
}
58+
panic("Developer error: failed to check auth:"+err.Error())
59+
}
60+
}
61+
62+
func (f*FakeEnqueuer)Enqueue(ctx context.Context,userID,templateID uuid.UUID,labelsmap[string]string,createdBystring,targets...uuid.UUID) (*uuid.UUID,error) {
63+
returnf.EnqueueWithData(ctx,userID,templateID,labels,nil,createdBy,targets...)
64+
}
65+
66+
func (f*FakeEnqueuer)EnqueueWithData(ctx context.Context,userID,templateID uuid.UUID,labelsmap[string]string,datamap[string]any,createdBystring,targets...uuid.UUID) (*uuid.UUID,error) {
67+
returnf.enqueueWithDataLock(ctx,userID,templateID,labels,data,createdBy,targets...)
68+
}
69+
70+
func (f*FakeEnqueuer)enqueueWithDataLock(ctx context.Context,userID,templateID uuid.UUID,labelsmap[string]string,datamap[string]any,createdBystring,targets...uuid.UUID) (*uuid.UUID,error) {
71+
f.mu.Lock()
72+
deferf.mu.Unlock()
73+
f.assertRBACNoLock(ctx)
74+
75+
f.sent=append(f.sent,&FakeNotification{
76+
UserID:userID,
77+
TemplateID:templateID,
78+
Labels:labels,
79+
Data:data,
80+
CreatedBy:createdBy,
81+
Targets:targets,
82+
})
83+
84+
id:=uuid.New()
85+
return&id,nil
86+
}
87+
88+
func (f*FakeEnqueuer)Clear() {
89+
f.mu.Lock()
90+
deferf.mu.Unlock()
91+
92+
f.sent=nil
93+
}
94+
95+
func (f*FakeEnqueuer)Sent() []*FakeNotification {
96+
f.mu.Lock()
97+
deferf.mu.Unlock()
98+
returnappend([]*FakeNotification{},f.sent...)
99+
}

0 commit comments

Comments
 (0)

[8]ページ先頭

©2009-2025 Movatter.jp