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

Commit7b09d98

Browse files
authored
chore: add /groups endpoint to filter byorganization and/ormember (#14260)
* chore: merge get groups sql queries into 1* Add endpoint for fetching groups with filters* remove 2 ways to customizing a fake authorizer
1 parent83ccdaa commit7b09d98

File tree

24 files changed

+537
-287
lines changed

24 files changed

+537
-287
lines changed

‎coderd/apidoc/docs.go

Lines changed: 44 additions & 0 deletions
Some generated files are not rendered by default. Learn more aboutcustomizing how changed files appear on GitHub.

‎coderd/apidoc/swagger.json

Lines changed: 40 additions & 0 deletions
Some generated files are not rendered by default. Learn more aboutcustomizing how changed files appear on GitHub.

‎coderd/coderdtest/authorize.go

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -353,16 +353,28 @@ func (s *PreparedRecorder) CompileToSQL(ctx context.Context, cfg regosql.Convert
353353
returns.prepped.CompileToSQL(ctx,cfg)
354354
}
355355

356-
// FakeAuthorizer is an Authorizer that always returns the same error.
356+
// FakeAuthorizer is an Authorizer that will return an error based on the
357+
// "ConditionalReturn" function. By default, **no error** is returned.
358+
// Meaning 'FakeAuthorizer' by default will never return "unauthorized".
357359
typeFakeAuthorizerstruct {
358-
// AlwaysReturn is the error that will be returned by Authorize.
359-
AlwaysReturnerror
360+
ConditionalReturnfunc(context.Context, rbac.Subject, policy.Action, rbac.Object)error
360361
}
361362

362363
var_ rbac.Authorizer= (*FakeAuthorizer)(nil)
363364

364-
func (d*FakeAuthorizer)Authorize(_ context.Context,_ rbac.Subject,_ policy.Action,_ rbac.Object)error {
365-
returnd.AlwaysReturn
365+
// AlwaysReturn is the error that will be returned by Authorize.
366+
func (d*FakeAuthorizer)AlwaysReturn(errerror)*FakeAuthorizer {
367+
d.ConditionalReturn=func(_ context.Context,_ rbac.Subject,_ policy.Action,_ rbac.Object)error {
368+
returnerr
369+
}
370+
returnd
371+
}
372+
373+
func (d*FakeAuthorizer)Authorize(ctx context.Context,subject rbac.Subject,action policy.Action,object rbac.Object)error {
374+
ifd.ConditionalReturn!=nil {
375+
returnd.ConditionalReturn(ctx,subject,action,object)
376+
}
377+
returnnil
366378
}
367379

368380
func (d*FakeAuthorizer)Prepare(_ context.Context,subject rbac.Subject,action policy.Action,_string) (rbac.PreparedAuthorized,error) {

‎coderd/database/dbauthz/dbauthz.go

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1491,19 +1491,16 @@ func (q *querier) GetGroupMembersCountByGroupID(ctx context.Context, groupID uui
14911491
returnmemberCount,nil
14921492
}
14931493

1494-
func (q*querier)GetGroups(ctx context.Context) ([]database.Group,error) {
1495-
iferr:=q.authorizeContext(ctx,policy.ActionRead,rbac.ResourceSystem);err!=nil {
1496-
returnnil,err
1494+
func (q*querier)GetGroups(ctx context.Context,arg database.GetGroupsParams) ([]database.Group,error) {
1495+
iferr:=q.authorizeContext(ctx,policy.ActionRead,rbac.ResourceSystem);err==nil {
1496+
// Optimize this query for system users as it is used in telemetry.
1497+
// Calling authz on all groups in a deployment for telemetry jobs is
1498+
// excessive. Most user calls should have some filtering applied to reduce
1499+
// the size of the set.
1500+
returnq.db.GetGroups(ctx,arg)
14971501
}
1498-
returnq.db.GetGroups(ctx)
1499-
}
1500-
1501-
func (q*querier)GetGroupsByOrganizationAndUserID(ctx context.Context,arg database.GetGroupsByOrganizationAndUserIDParams) ([]database.Group,error) {
1502-
returnfetchWithPostFilter(q.auth,policy.ActionRead,q.db.GetGroupsByOrganizationAndUserID)(ctx,arg)
1503-
}
15041502

1505-
func (q*querier)GetGroupsByOrganizationID(ctx context.Context,organizationID uuid.UUID) ([]database.Group,error) {
1506-
returnfetchWithPostFilter(q.auth,policy.ActionRead,q.db.GetGroupsByOrganizationID)(ctx,organizationID)
1503+
returnfetchWithPostFilter(q.auth,policy.ActionRead,q.db.GetGroups)(ctx,arg)
15071504
}
15081505

15091506
func (q*querier)GetHealthSettings(ctx context.Context) (string,error) {

‎coderd/database/dbauthz/dbauthz_test.go

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ func TestInTX(t *testing.T) {
8181

8282
db:=dbmem.New()
8383
q:=dbauthz.New(db,&coderdtest.RecordingAuthorizer{
84-
Wrapped:&coderdtest.FakeAuthorizer{AlwaysReturn:xerrors.New("custom error")},
84+
Wrapped:(&coderdtest.FakeAuthorizer{}).AlwaysReturn(xerrors.New("custom error")),
8585
},slog.Make(),coderdtest.AccessControlStorePointer())
8686
actor:= rbac.Subject{
8787
ID:uuid.NewString(),
@@ -110,7 +110,7 @@ func TestNew(t *testing.T) {
110110
db=dbmem.New()
111111
exp=dbgen.Workspace(t,db, database.Workspace{})
112112
rec=&coderdtest.RecordingAuthorizer{
113-
Wrapped:&coderdtest.FakeAuthorizer{AlwaysReturn:nil},
113+
Wrapped:&coderdtest.FakeAuthorizer{},
114114
}
115115
subj= rbac.Subject{}
116116
ctx=dbauthz.As(context.Background(), rbac.Subject{})
@@ -135,7 +135,7 @@ func TestNew(t *testing.T) {
135135
funcTestDBAuthzRecursive(t*testing.T) {
136136
t.Parallel()
137137
q:=dbauthz.New(dbmem.New(),&coderdtest.RecordingAuthorizer{
138-
Wrapped:&coderdtest.FakeAuthorizer{AlwaysReturn:nil},
138+
Wrapped:&coderdtest.FakeAuthorizer{},
139139
},slog.Make(),coderdtest.AccessControlStorePointer())
140140
actor:= rbac.Subject{
141141
ID:uuid.NewString(),
@@ -342,18 +342,21 @@ func (s *MethodTestSuite) TestGroup() {
342342
dbgen.GroupMember(s.T(),db, database.GroupMemberTable{GroupID:g.ID,UserID:u.ID})
343343
check.Asserts(rbac.ResourceSystem,policy.ActionRead)
344344
}))
345-
s.Run("GetGroups",s.Subtest(func(db database.Store,check*expects) {
345+
s.Run("System/GetGroups",s.Subtest(func(db database.Store,check*expects) {
346346
_=dbgen.Group(s.T(),db, database.Group{})
347-
check.Asserts(rbac.ResourceSystem,policy.ActionRead)
347+
check.Args(database.GetGroupsParams{}).
348+
Asserts(rbac.ResourceSystem,policy.ActionRead)
348349
}))
349-
s.Run("GetGroupsByOrganizationAndUserID",s.Subtest(func(db database.Store,check*expects) {
350+
s.Run("GetGroups",s.Subtest(func(db database.Store,check*expects) {
350351
g:=dbgen.Group(s.T(),db, database.Group{})
351352
u:=dbgen.User(s.T(),db, database.User{})
352353
gm:=dbgen.GroupMember(s.T(),db, database.GroupMemberTable{GroupID:g.ID,UserID:u.ID})
353-
check.Args(database.GetGroupsByOrganizationAndUserIDParams{
354+
check.Args(database.GetGroupsParams{
354355
OrganizationID:g.OrganizationID,
355-
UserID:gm.UserID,
356-
}).Asserts(g,policy.ActionRead)
356+
HasMemberID:gm.UserID,
357+
}).Asserts(rbac.ResourceSystem,policy.ActionRead,g,policy.ActionRead).
358+
// Fail the system resource skip
359+
FailSystemObjectChecks()
357360
}))
358361
s.Run("InsertAllUsersGroup",s.Subtest(func(db database.Store,check*expects) {
359362
o:=dbgen.Organization(s.T(),db, database.Organization{})
@@ -597,12 +600,16 @@ func (s *MethodTestSuite) TestLicense() {
597600
}
598601

599602
func (s*MethodTestSuite)TestOrganization() {
600-
s.Run("GetGroupsByOrganizationID",s.Subtest(func(db database.Store,check*expects) {
603+
s.Run("ByOrganization/GetGroups",s.Subtest(func(db database.Store,check*expects) {
601604
o:=dbgen.Organization(s.T(),db, database.Organization{})
602605
a:=dbgen.Group(s.T(),db, database.Group{OrganizationID:o.ID})
603606
b:=dbgen.Group(s.T(),db, database.Group{OrganizationID:o.ID})
604-
check.Args(o.ID).Asserts(a,policy.ActionRead,b,policy.ActionRead).
605-
Returns([]database.Group{a,b})
607+
check.Args(database.GetGroupsParams{
608+
OrganizationID:o.ID,
609+
}).Asserts(rbac.ResourceSystem,policy.ActionRead,a,policy.ActionRead,b,policy.ActionRead).
610+
Returns([]database.Group{a,b}).
611+
// Fail the system check shortcut
612+
FailSystemObjectChecks()
606613
}))
607614
s.Run("GetOrganizationByID",s.Subtest(func(db database.Store,check*expects) {
608615
o:=dbgen.Organization(s.T(),db, database.Organization{})

‎coderd/database/dbauthz/setup_test.go

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -114,9 +114,7 @@ func (s *MethodTestSuite) Subtest(testCaseF func(db database.Store, check *expec
114114
s.methodAccounting[methodName]++
115115

116116
db:=dbmem.New()
117-
fakeAuthorizer:=&coderdtest.FakeAuthorizer{
118-
AlwaysReturn:nil,
119-
}
117+
fakeAuthorizer:=&coderdtest.FakeAuthorizer{}
120118
rec:=&coderdtest.RecordingAuthorizer{
121119
Wrapped:fakeAuthorizer,
122120
}
@@ -174,7 +172,11 @@ func (s *MethodTestSuite) Subtest(testCaseF func(db database.Store, check *expec
174172
// Always run
175173
s.Run("Success",func() {
176174
rec.Reset()
177-
fakeAuthorizer.AlwaysReturn=nil
175+
iftestCase.successAuthorizer!=nil {
176+
fakeAuthorizer.ConditionalReturn=testCase.successAuthorizer
177+
}else {
178+
fakeAuthorizer.AlwaysReturn(nil)
179+
}
178180

179181
outputs,err:=callMethod(ctx)
180182
iftestCase.err==nil {
@@ -232,7 +234,7 @@ func (s *MethodTestSuite) NoActorErrorTest(callMethod func(ctx context.Context)
232234
// Asserts that the error returned is a NotAuthorizedError.
233235
func (s*MethodTestSuite)NotAuthorizedErrorTest(ctx context.Context,az*coderdtest.FakeAuthorizer,testCaseexpects,callMethodfunc(ctx context.Context) ([]reflect.Value,error)) {
234236
s.Run("NotAuthorized",func() {
235-
az.AlwaysReturn=rbac.ForbiddenWithInternal(xerrors.New("Always fail authz"), rbac.Subject{},"", rbac.Object{},nil)
237+
az.AlwaysReturn(rbac.ForbiddenWithInternal(xerrors.New("Always fail authz"), rbac.Subject{},"", rbac.Object{},nil))
236238

237239
// If we have assertions, that means the method should FAIL
238240
// if RBAC will disallow the request. The returned error should
@@ -257,8 +259,8 @@ func (s *MethodTestSuite) NotAuthorizedErrorTest(ctx context.Context, az *coderd
257259
// Pass in a canceled context
258260
ctx,cancel:=context.WithCancel(ctx)
259261
cancel()
260-
az.AlwaysReturn=rbac.ForbiddenWithInternal(&topdown.Error{Code:topdown.CancelErr},
261-
rbac.Subject{},"", rbac.Object{},nil)
262+
az.AlwaysReturn(rbac.ForbiddenWithInternal(&topdown.Error{Code:topdown.CancelErr},
263+
rbac.Subject{},"", rbac.Object{},nil))
262264

263265
// If we have assertions, that means the method should FAIL
264266
// if RBAC will disallow the request. The returned error should
@@ -324,6 +326,7 @@ type expects struct {
324326
// instead.
325327
notAuthorizedExpectstring
326328
cancelledCtxExpectstring
329+
successAuthorizerfunc(ctx context.Context,subject rbac.Subject,action policy.Action,obj rbac.Object)error
327330
}
328331

329332
// Asserts is required. Asserts the RBAC authorize calls that should be made.
@@ -354,6 +357,23 @@ func (m *expects) Errors(err error) *expects {
354357
returnm
355358
}
356359

360+
func (m*expects)FailSystemObjectChecks()*expects {
361+
returnm.WithSuccessAuthorizer(func(ctx context.Context,subject rbac.Subject,action policy.Action,obj rbac.Object)error {
362+
ifobj.Type==rbac.ResourceSystem.Type {
363+
returnxerrors.Errorf("hard coded system authz failed")
364+
}
365+
returnnil
366+
})
367+
}
368+
369+
// WithSuccessAuthorizer is helpful when an optimization authz check is made
370+
// to skip some RBAC checks. This check in testing would prevent the ability
371+
// to assert the more nuanced RBAC checks.
372+
func (m*expects)WithSuccessAuthorizer(ffunc(ctx context.Context,subject rbac.Subject,action policy.Action,obj rbac.Object)error)*expects {
373+
m.successAuthorizer=f
374+
returnm
375+
}
376+
357377
func (m*expects)WithNotAuthorized(containsstring)*expects {
358378
m.notAuthorizedExpect=contains
359379
returnm

‎coderd/database/dbmem/dbmem.go

Lines changed: 25 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -2599,51 +2599,46 @@ func (q *FakeQuerier) GetGroupMembersCountByGroupID(ctx context.Context, groupID
25992599
returnint64(len(users)),nil
26002600
}
26012601

2602-
func (q*FakeQuerier)GetGroups(_ context.Context) ([]database.Group,error) {
2603-
q.mutex.RLock()
2604-
deferq.mutex.RUnlock()
2605-
2606-
out:=make([]database.Group,len(q.groups))
2607-
copy(out,q.groups)
2608-
returnout,nil
2609-
}
2610-
2611-
func (q*FakeQuerier)GetGroupsByOrganizationAndUserID(_ context.Context,arg database.GetGroupsByOrganizationAndUserIDParams) ([]database.Group,error) {
2602+
func (q*FakeQuerier)GetGroups(_ context.Context,arg database.GetGroupsParams) ([]database.Group,error) {
26122603
err:=validateDatabaseType(arg)
26132604
iferr!=nil {
26142605
returnnil,err
26152606
}
26162607

26172608
q.mutex.RLock()
26182609
deferq.mutex.RUnlock()
2619-
vargroupIDs []uuid.UUID
2620-
for_,member:=rangeq.groupMembers {
2621-
ifmember.UserID==arg.UserID {
2622-
groupIDs=append(groupIDs,member.GroupID)
2610+
2611+
groupIDs:=make(map[uuid.UUID]struct{})
2612+
ifarg.HasMemberID!=uuid.Nil {
2613+
for_,member:=rangeq.groupMembers {
2614+
ifmember.UserID==arg.HasMemberID {
2615+
groupIDs[member.GroupID]=struct{}{}
2616+
}
26232617
}
2624-
}
2625-
groups:= []database.Group{}
2626-
for_,group:=rangeq.groups {
2627-
ifslices.Contains(groupIDs,group.ID)&&group.OrganizationID==arg.OrganizationID {
2628-
groups=append(groups,group)
2618+
2619+
// Handle the everyone group
2620+
for_,orgMember:=rangeq.organizationMembers {
2621+
iforgMember.UserID==arg.HasMemberID {
2622+
groupIDs[orgMember.OrganizationID]=struct{}{}
2623+
}
26292624
}
26302625
}
26312626

2632-
returngroups,nil
2633-
}
2634-
2635-
func (q*FakeQuerier)GetGroupsByOrganizationID(_ context.Context,id uuid.UUID) ([]database.Group,error) {
2636-
q.mutex.RLock()
2637-
deferq.mutex.RUnlock()
2638-
2639-
groups:=make([]database.Group,0,len(q.groups))
2627+
filtered:=make([]database.Group,0)
26402628
for_,group:=rangeq.groups {
2641-
ifgroup.OrganizationID==id {
2642-
groups=append(groups,group)
2629+
ifarg.OrganizationID!=uuid.Nil&&group.OrganizationID!=arg.OrganizationID {
2630+
continue
2631+
}
2632+
2633+
_,ok:=groupIDs[group.ID]
2634+
ifarg.HasMemberID!=uuid.Nil&&!ok {
2635+
continue
26432636
}
2637+
2638+
filtered=append(filtered,group)
26442639
}
26452640

2646-
returngroups,nil
2641+
returnfiltered,nil
26472642
}
26482643

26492644
func (q*FakeQuerier)GetHealthSettings(_ context.Context) (string,error) {

0 commit comments

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