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

Commitb909207

Browse files
committed
chore: unit tests for cache authz
1 parent496a096 commitb909207

File tree

7 files changed

+148
-13
lines changed

7 files changed

+148
-13
lines changed

‎coderd/coderd.go

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -572,13 +572,10 @@ func New(options *Options) *API {
572572
TemplateScheduleStore:options.TemplateScheduleStore,
573573
UserQuietHoursScheduleStore:options.UserQuietHoursScheduleStore,
574574
AccessControlStore:options.AccessControlStore,
575-
FileCache:files.NewFromStore(options.Database,options.PrometheusRegistry,func(ctx context.Context,action policy.Action,object rbac.Object)error {
576-
subject:=httpmw.UserAuthorizationCtx(ctx)
577-
returnoptions.Authorizer.Authorize(ctx,subject,action,object)
578-
}),
579-
Experiments:experiments,
580-
WebpushDispatcher:options.WebPushDispatcher,
581-
healthCheckGroup:&singleflight.Group[string,*healthsdk.HealthcheckReport]{},
575+
FileCache:files.NewFromStore(options.Database,options.PrometheusRegistry,options.Authorizer.Authorize),
576+
Experiments:experiments,
577+
WebpushDispatcher:options.WebPushDispatcher,
578+
healthCheckGroup:&singleflight.Group[string,*healthsdk.HealthcheckReport]{},
582579
Acquirer:provisionerdserver.NewAcquirer(
583580
ctx,
584581
options.Logger.Named("acquirer"),

‎coderd/coderdtest/authorize.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,10 @@ func (r *RecordingAuthorizer) AssertOutOfOrder(t *testing.T, actor rbac.Subject,
234234
// AssertActor asserts in order. If the order of authz calls does not match,
235235
// this will fail.
236236
func (r*RecordingAuthorizer)AssertActor(t*testing.T,actor rbac.Subject,did...ActionObjectPair) {
237+
r.AssertActorID(t,actor.ID,did...)
238+
}
239+
240+
func (r*RecordingAuthorizer)AssertActorID(t*testing.T,idstring,did...ActionObjectPair) {
237241
r.Lock()
238242
deferr.Unlock()
239243
ptr:=0
@@ -242,7 +246,7 @@ func (r *RecordingAuthorizer) AssertActor(t *testing.T, actor rbac.Subject, did
242246
// Finished all assertions
243247
return
244248
}
245-
ifcall.Actor.ID==actor.ID {
249+
ifcall.Actor.ID==id {
246250
action,object:=did[ptr].Action,did[ptr].Object
247251
assert.Equalf(t,action,call.Action,"assert action %d",ptr)
248252
assert.Equalf(t,object,call.Object,"assert object %d",ptr)

‎coderd/database/dbauthz/dbauthz.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -436,7 +436,8 @@ var (
436436
subjectFileReader= rbac.Subject{
437437
Type:rbac.SubjectTypeFileReader,
438438
FriendlyName:"Can Read All Files",
439-
ID:uuid.Nil.String(),
439+
// Arbitrary uuid to have a unique ID for this subject.
440+
ID:rbac.SubjectTypeFileReaderID,
440441
Roles:rbac.Roles([]rbac.Role{
441442
{
442443
Identifier: rbac.RoleIdentifier{Name:"file-reader"},

‎coderd/files/cache.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import (
1919
"github.com/coder/coder/v2/coderd/util/lazy"
2020
)
2121

22-
typeAuthorizeFilefunc(ctx context.Context,action policy.Action,object rbac.Object)error
22+
typeAuthorizeFilefunc(ctx context.Context,subject rbac.Subject,action policy.Action,object rbac.Object)error
2323

2424
// NewFromStore returns a file cache that will fetch files from the provided
2525
// database.
@@ -158,8 +158,12 @@ func (c *Cache) Acquire(ctx context.Context, fileID uuid.UUID) (fs.FS, error) {
158158
returnnil,err
159159
}
160160

161+
subject,ok:=dbauthz.ActorFromContext(ctx)
162+
if!ok {
163+
returnnil,dbauthz.ErrNoActor
164+
}
161165
// Always check the caller can actually read the file.
162-
ifc.authz(ctx,policy.ActionRead,it.object)!=nil {
166+
iferr:=c.authz(ctx,subject,policy.ActionRead,it.object);err!=nil {
163167
c.Release(fileID)
164168
returnnil,err
165169
}

‎coderd/files/cache_internal_test.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,15 @@ import (
1313
"golang.org/x/sync/errgroup"
1414

1515
"github.com/coder/coder/v2/coderd/coderdtest/promhelp"
16+
"github.com/coder/coder/v2/coderd/rbac"
17+
"github.com/coder/coder/v2/coderd/rbac/policy"
1618
"github.com/coder/coder/v2/testutil"
1719
)
1820

21+
funcauthzAlwaysTrue(_ context.Context,_ rbac.Subject,_ policy.Action,_ rbac.Object)error {
22+
returnnil
23+
}
24+
1925
funccachePromMetricName(metricstring)string {
2026
return"coderd_file_cache_"+metric
2127
}
@@ -33,7 +39,7 @@ func TestConcurrency(t *testing.T) {
3339
// will be waiting in line, ensuring that no one duplicated a fetch.
3440
time.Sleep(testutil.IntervalMedium)
3541
returncacheEntryValue{FS:emptyFS,size:fileSize},nil
36-
},reg)
42+
},reg,authzAlwaysTrue)
3743

3844
batches:=1000
3945
groups:=make([]*errgroup.Group,0,batches)
@@ -83,7 +89,7 @@ func TestRelease(t *testing.T) {
8389
FS:emptyFS,
8490
size:fileSize,
8591
},nil
86-
},reg)
92+
},reg,authzAlwaysTrue)
8793

8894
batches:=100
8995
ids:=make([]uuid.UUID,0,batches)

‎coderd/files/cache_test.go

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
package files_test
2+
3+
import (
4+
"testing"
5+
6+
"github.com/google/uuid"
7+
"github.com/prometheus/client_golang/prometheus"
8+
"github.com/stretchr/testify/require"
9+
10+
"cdr.dev/slog/sloggers/slogtest"
11+
"github.com/coder/coder/v2/coderd/coderdtest"
12+
"github.com/coder/coder/v2/coderd/database"
13+
"github.com/coder/coder/v2/coderd/database/dbauthz"
14+
"github.com/coder/coder/v2/coderd/database/dbgen"
15+
"github.com/coder/coder/v2/coderd/database/dbtestutil"
16+
"github.com/coder/coder/v2/coderd/files"
17+
"github.com/coder/coder/v2/coderd/rbac"
18+
"github.com/coder/coder/v2/coderd/rbac/policy"
19+
"github.com/coder/coder/v2/testutil"
20+
)
21+
22+
funcTestCacheRBAC(t*testing.T) {
23+
t.Parallel()
24+
25+
db,cache,rec:=cacheAuthzSetup(t)
26+
ctx:=testutil.Context(t,testutil.WaitMedium)
27+
28+
file:=dbgen.File(t,db, database.File{})
29+
30+
nobodyID:=uuid.New()
31+
nobody:=dbauthz.As(ctx, rbac.Subject{
32+
ID:nobodyID.String(),
33+
Roles: rbac.Roles{},
34+
Scope:rbac.ScopeAll,
35+
})
36+
37+
userID:=uuid.New()
38+
userReader:=dbauthz.As(ctx, rbac.Subject{
39+
ID:userID.String(),
40+
Roles: rbac.Roles{
41+
must(rbac.RoleByName(rbac.RoleTemplateAdmin())),
42+
},
43+
Scope:rbac.ScopeAll,
44+
})
45+
46+
cacheReader:=dbauthz.AsFileReader(ctx)
47+
48+
t.Run("NoRolesOpen",func(t*testing.T) {
49+
// Ensure start is clean
50+
require.Equal(t,0,cache.Count())
51+
rec.Reset()
52+
53+
_,err:=cache.Acquire(nobody,file.ID)
54+
require.Error(t,err)
55+
require.True(t,rbac.IsUnauthorizedError(err))
56+
57+
// Ensure that the cache is empty
58+
require.Equal(t,0,cache.Count())
59+
60+
// Check the assertions
61+
rec.AssertActorID(t,nobodyID.String(),rec.Pair(policy.ActionRead,file))
62+
rec.AssertActorID(t,rbac.SubjectTypeFileReaderID,rec.Pair(policy.ActionRead,file))
63+
})
64+
65+
t.Run("CacheHasFile",func(t*testing.T) {
66+
rec.Reset()
67+
require.Equal(t,0,cache.Count())
68+
69+
// Read the file with a file reader to put it into the cache.
70+
_,err:=cache.Acquire(cacheReader,file.ID)
71+
require.NoError(t,err)
72+
require.Equal(t,1,cache.Count())
73+
74+
// "nobody" should not be able to read the file.
75+
_,err=cache.Acquire(nobody,file.ID)
76+
require.Error(t,err)
77+
require.True(t,rbac.IsUnauthorizedError(err))
78+
require.Equal(t,1,cache.Count())
79+
80+
// UserReader can
81+
_,err=cache.Acquire(userReader,file.ID)
82+
require.NoError(t,err)
83+
require.Equal(t,1,cache.Count())
84+
85+
cache.Release(file.ID)
86+
cache.Release(file.ID)
87+
require.Equal(t,0,cache.Count())
88+
89+
rec.AssertActorID(t,nobodyID.String(),rec.Pair(policy.ActionRead,file))
90+
rec.AssertActorID(t,rbac.SubjectTypeFileReaderID,rec.Pair(policy.ActionRead,file))
91+
rec.AssertActorID(t,userID.String(),rec.Pair(policy.ActionRead,file))
92+
})
93+
}
94+
95+
funccacheAuthzSetup(t*testing.T) (database.Store,*files.Cache,*coderdtest.RecordingAuthorizer) {
96+
t.Helper()
97+
98+
logger:=slogtest.Make(t,&slogtest.Options{})
99+
reg:=prometheus.NewRegistry()
100+
101+
db,_:=dbtestutil.NewDB(t)
102+
authz:=rbac.NewAuthorizer(reg)
103+
rec:=&coderdtest.RecordingAuthorizer{
104+
Called:nil,
105+
Wrapped:authz,
106+
}
107+
108+
// Dbauthz wrap the db
109+
db=dbauthz.New(db,rec,logger,coderdtest.AccessControlStorePointer())
110+
c:=files.NewFromStore(db,reg,rec.Authorize)
111+
returndb,c,rec
112+
}
113+
114+
funcmust[Tany](tT,errerror)T {
115+
iferr!=nil {
116+
panic(err)
117+
}
118+
returnt
119+
}

‎coderd/rbac/authz.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,10 @@ const (
7777
SubjectTypeFileReaderSubjectType="file_reader"
7878
)
7979

80+
const (
81+
SubjectTypeFileReaderID="acbf0be6-6fed-47b6-8c43-962cb5cab994"
82+
)
83+
8084
// Subject is a struct that contains all the elements of a subject in an rbac
8185
// authorize.
8286
typeSubjectstruct {

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp