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

Commitde9e688

Browse files
authored
chore: merge organization member db queries (#13542)
Merge members queries into 1 that also joins in the user table for username.Required to list organization members on UI/cli
1 parent1ca5dc0 commitde9e688

File tree

18 files changed

+290
-211
lines changed

18 files changed

+290
-211
lines changed

‎cli/server_createadminuser_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,12 +67,12 @@ func TestServerCreateAdminUser(t *testing.T) {
6767
orgIDs[org.ID]=struct{}{}
6868
}
6969

70-
orgMemberships,err:=db.GetOrganizationMembershipsByUserID(ctx,user.ID)
70+
orgMemberships,err:=db.OrganizationMembers(ctx,database.OrganizationMembersParams{UserID:user.ID})
7171
require.NoError(t,err)
7272
orgIDs2:=make(map[uuid.UUID]struct{},len(orgMemberships))
7373
for_,membership:=rangeorgMemberships {
74-
orgIDs2[membership.OrganizationID]=struct{}{}
75-
assert.Equal(t, []string{rbac.RoleOrgAdmin()},membership.Roles,"user is not org admin")
74+
orgIDs2[membership.OrganizationMember.OrganizationID]=struct{}{}
75+
assert.Equal(t, []string{rbac.RoleOrgAdmin()},membership.OrganizationMember.Roles,"user is not org admin")
7676
}
7777

7878
require.Equal(t,orgIDs,orgIDs2,"user is not in all orgs")

‎coderd/database/dbauthz/dbauthz.go

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1476,14 +1476,6 @@ func (q *querier) GetOrganizationIDsByMemberIDs(ctx context.Context, ids []uuid.
14761476
returnfetchWithPostFilter(q.auth,policy.ActionRead,q.db.GetOrganizationIDsByMemberIDs)(ctx,ids)
14771477
}
14781478

1479-
func (q*querier)GetOrganizationMemberByUserID(ctx context.Context,arg database.GetOrganizationMemberByUserIDParams) (database.OrganizationMember,error) {
1480-
returnfetch(q.log,q.auth,q.db.GetOrganizationMemberByUserID)(ctx,arg)
1481-
}
1482-
1483-
func (q*querier)GetOrganizationMembershipsByUserID(ctx context.Context,userID uuid.UUID) ([]database.OrganizationMember,error) {
1484-
returnfetchWithPostFilter(q.auth,policy.ActionRead,q.db.GetOrganizationMembershipsByUserID)(ctx,userID)
1485-
}
1486-
14871479
func (q*querier)GetOrganizations(ctx context.Context) ([]database.Organization,error) {
14881480
fetch:=func(ctx context.Context,_interface{}) ([]database.Organization,error) {
14891481
returnq.db.GetOrganizations(ctx)
@@ -2771,6 +2763,10 @@ func (q *querier) ListWorkspaceAgentPortShares(ctx context.Context, workspaceID
27712763
returnq.db.ListWorkspaceAgentPortShares(ctx,workspaceID)
27722764
}
27732765

2766+
func (q*querier)OrganizationMembers(ctx context.Context,arg database.OrganizationMembersParams) ([]database.OrganizationMembersRow,error) {
2767+
returnfetchWithPostFilter(q.auth,policy.ActionRead,q.db.OrganizationMembers)(ctx,arg)
2768+
}
2769+
27742770
func (q*querier)ReduceWorkspaceAgentShareLevelToAuthenticatedByTemplate(ctx context.Context,templateID uuid.UUID)error {
27752771
template,err:=q.db.GetTemplateByID(ctx,templateID)
27762772
iferr!=nil {
@@ -2870,15 +2866,15 @@ func (q *querier) UpdateInactiveUsersToDormant(ctx context.Context, lastSeenAfte
28702866

28712867
func (q*querier)UpdateMemberRoles(ctx context.Context,arg database.UpdateMemberRolesParams) (database.OrganizationMember,error) {
28722868
// Authorized fetch will check that the actor has read access to the org member since the org member is returned.
2873-
member,err:=q.GetOrganizationMemberByUserID(ctx, database.GetOrganizationMemberByUserIDParams{
2869+
member,err:=database.ExpectOne(q.OrganizationMembers(ctx, database.OrganizationMembersParams{
28742870
OrganizationID:arg.OrgID,
28752871
UserID:arg.UserID,
2876-
})
2872+
}))
28772873
iferr!=nil {
28782874
return database.OrganizationMember{},err
28792875
}
28802876

2881-
originalRoles,err:=q.convertToOrganizationRoles(member.OrganizationID,member.Roles)
2877+
originalRoles,err:=q.convertToOrganizationRoles(member.OrganizationMember.OrganizationID,member.OrganizationMember.Roles)
28822878
iferr!=nil {
28832879
return database.OrganizationMember{},xerrors.Errorf("convert original roles: %w",err)
28842880
}

‎coderd/database/dbauthz/dbauthz_test.go

Lines changed: 24 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -596,19 +596,6 @@ func (s *MethodTestSuite) TestOrganization() {
596596
check.Args([]uuid.UUID{ma.UserID,mb.UserID}).
597597
Asserts(rbac.ResourceUserObject(ma.UserID),policy.ActionRead,rbac.ResourceUserObject(mb.UserID),policy.ActionRead)
598598
}))
599-
s.Run("GetOrganizationMemberByUserID",s.Subtest(func(db database.Store,check*expects) {
600-
mem:=dbgen.OrganizationMember(s.T(),db, database.OrganizationMember{})
601-
check.Args(database.GetOrganizationMemberByUserIDParams{
602-
OrganizationID:mem.OrganizationID,
603-
UserID:mem.UserID,
604-
}).Asserts(mem,policy.ActionRead).Returns(mem)
605-
}))
606-
s.Run("GetOrganizationMembershipsByUserID",s.Subtest(func(db database.Store,check*expects) {
607-
u:=dbgen.User(s.T(),db, database.User{})
608-
a:=dbgen.OrganizationMember(s.T(),db, database.OrganizationMember{UserID:u.ID})
609-
b:=dbgen.OrganizationMember(s.T(),db, database.OrganizationMember{UserID:u.ID})
610-
check.Args(u.ID).Asserts(a,policy.ActionRead,b,policy.ActionRead).Returns(slice.New(a,b))
611-
}))
612599
s.Run("GetOrganizations",s.Subtest(func(db database.Store,check*expects) {
613600
def,_:=db.GetDefaultOrganization(context.Background())
614601
a:=dbgen.Organization(s.T(),db, database.Organization{})
@@ -658,6 +645,22 @@ func (s *MethodTestSuite) TestOrganization() {
658645
o.ID,
659646
).Asserts(o,policy.ActionDelete)
660647
}))
648+
s.Run("OrganizationMembers",s.Subtest(func(db database.Store,check*expects) {
649+
o:=dbgen.Organization(s.T(),db, database.Organization{})
650+
u:=dbgen.User(s.T(),db, database.User{})
651+
mem:=dbgen.OrganizationMember(s.T(),db, database.OrganizationMember{
652+
OrganizationID:o.ID,
653+
UserID:u.ID,
654+
Roles: []string{rbac.RoleOrgAdmin()},
655+
})
656+
657+
check.Args(database.OrganizationMembersParams{
658+
OrganizationID: uuid.UUID{},
659+
UserID: uuid.UUID{},
660+
}).Asserts(
661+
mem,policy.ActionRead,
662+
)
663+
}))
661664
s.Run("UpdateMemberRoles",s.Subtest(func(db database.Store,check*expects) {
662665
o:=dbgen.Organization(s.T(),db, database.Organization{})
663666
u:=dbgen.User(s.T(),db, database.User{})
@@ -673,11 +676,14 @@ func (s *MethodTestSuite) TestOrganization() {
673676
GrantedRoles: []string{},
674677
UserID:u.ID,
675678
OrgID:o.ID,
676-
}).Asserts(
677-
mem,policy.ActionRead,
678-
rbac.ResourceAssignRole.InOrg(o.ID),policy.ActionAssign,// org-mem
679-
rbac.ResourceAssignRole.InOrg(o.ID),policy.ActionDelete,// org-admin
680-
).Returns(out)
679+
}).
680+
WithNotAuthorized(sql.ErrNoRows.Error()).
681+
WithCancelled(sql.ErrNoRows.Error()).
682+
Asserts(
683+
mem,policy.ActionRead,
684+
rbac.ResourceAssignRole.InOrg(o.ID),policy.ActionAssign,// org-mem
685+
rbac.ResourceAssignRole.InOrg(o.ID),policy.ActionDelete,// org-admin
686+
).Returns(out)
681687
}))
682688
}
683689

‎coderd/database/dbauthz/setup_test.go

Lines changed: 33 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ func (s *MethodTestSuite) Subtest(testCaseF func(db database.Store, check *expec
157157
iflen(testCase.assertions)>0 {
158158
// Only run these tests if we know the underlying call makes
159159
// rbac assertions.
160-
s.NotAuthorizedErrorTest(ctx,fakeAuthorizer,callMethod)
160+
s.NotAuthorizedErrorTest(ctx,fakeAuthorizer,testCase,callMethod)
161161
}
162162

163163
iflen(testCase.assertions)>0||
@@ -230,7 +230,7 @@ func (s *MethodTestSuite) NoActorErrorTest(callMethod func(ctx context.Context)
230230

231231
// NotAuthorizedErrorTest runs the given method with an authorizer that will fail authz.
232232
// Asserts that the error returned is a NotAuthorizedError.
233-
func (s*MethodTestSuite)NotAuthorizedErrorTest(ctx context.Context,az*coderdtest.FakeAuthorizer,callMethodfunc(ctx context.Context) ([]reflect.Value,error)) {
233+
func (s*MethodTestSuite)NotAuthorizedErrorTest(ctx context.Context,az*coderdtest.FakeAuthorizer,testCaseexpects,callMethodfunc(ctx context.Context) ([]reflect.Value,error)) {
234234
s.Run("NotAuthorized",func() {
235235
az.AlwaysReturn=rbac.ForbiddenWithInternal(xerrors.New("Always fail authz"), rbac.Subject{},"", rbac.Object{},nil)
236236

@@ -242,9 +242,14 @@ func (s *MethodTestSuite) NotAuthorizedErrorTest(ctx context.Context, az *coderd
242242
// This is unfortunate, but if we are using `Filter` the error returned will be nil. So filter out
243243
// any case where the error is nil and the response is an empty slice.
244244
iferr!=nil||!hasEmptySliceResponse(resp) {
245-
s.ErrorContainsf(err,"unauthorized","error string should have a good message")
246-
s.Errorf(err,"method should an error with disallow authz")
247-
s.ErrorAs(err,&dbauthz.NotAuthorizedError{},"error should be NotAuthorizedError")
245+
// Expect the default error
246+
iftestCase.notAuthorizedExpect=="" {
247+
s.ErrorContainsf(err,"unauthorized","error string should have a good message")
248+
s.Errorf(err,"method should an error with disallow authz")
249+
s.ErrorAs(err,&dbauthz.NotAuthorizedError{},"error should be NotAuthorizedError")
250+
}else {
251+
s.ErrorContains(err,testCase.notAuthorizedExpect)
252+
}
248253
}
249254
})
250255

@@ -263,8 +268,12 @@ func (s *MethodTestSuite) NotAuthorizedErrorTest(ctx context.Context, az *coderd
263268
// This is unfortunate, but if we are using `Filter` the error returned will be nil. So filter out
264269
// any case where the error is nil and the response is an empty slice.
265270
iferr!=nil||!hasEmptySliceResponse(resp) {
266-
s.Errorf(err,"method should an error with cancellation")
267-
s.ErrorIsf(err,context.Canceled,"error should match context.Canceled")
271+
iftestCase.cancelledCtxExpect=="" {
272+
s.Errorf(err,"method should an error with cancellation")
273+
s.ErrorIsf(err,context.Canceled,"error should match context.Canceled")
274+
}else {
275+
s.ErrorContains(err,testCase.cancelledCtxExpect)
276+
}
268277
}
269278
})
270279
}
@@ -308,6 +317,13 @@ type expects struct {
308317
// outputs is optional. Can assert non-error return values.
309318
outputs []reflect.Value
310319
errerror
320+
321+
// Optional override of the default error checks.
322+
// By default, we search for the expected error strings.
323+
// If these strings are present, these strings will be searched
324+
// instead.
325+
notAuthorizedExpectstring
326+
cancelledCtxExpectstring
311327
}
312328

313329
// Asserts is required. Asserts the RBAC authorize calls that should be made.
@@ -338,6 +354,16 @@ func (m *expects) Errors(err error) *expects {
338354
returnm
339355
}
340356

357+
func (m*expects)WithNotAuthorized(containsstring)*expects {
358+
m.notAuthorizedExpect=contains
359+
returnm
360+
}
361+
362+
func (m*expects)WithCancelled(containsstring)*expects {
363+
m.cancelledCtxExpect=contains
364+
returnm
365+
}
366+
341367
// AssertRBAC contains the object and actions to be asserted.
342368
typeAssertRBACstruct {
343369
Object rbac.Object

‎coderd/database/dbgen/dbgen_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -119,10 +119,10 @@ func TestGenerator(t *testing.T) {
119119
t.Parallel()
120120
db:=dbmem.New()
121121
exp:=dbgen.OrganizationMember(t,db, database.OrganizationMember{})
122-
require.Equal(t,exp,must(db.GetOrganizationMemberByUserID(context.Background(), database.GetOrganizationMemberByUserIDParams{
122+
require.Equal(t,exp,must(database.ExpectOne(db.OrganizationMembers(context.Background(), database.OrganizationMembersParams{
123123
OrganizationID:exp.OrganizationID,
124124
UserID:exp.UserID,
125-
})))
125+
}))).OrganizationMember)
126126
})
127127

128128
t.Run("Workspace",func(t*testing.T) {

‎coderd/database/dbmem/dbmem.go

Lines changed: 28 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -2760,41 +2760,6 @@ func (q *FakeQuerier) GetOrganizationIDsByMemberIDs(_ context.Context, ids []uui
27602760
returngetOrganizationIDsByMemberIDRows,nil
27612761
}
27622762

2763-
func (q*FakeQuerier)GetOrganizationMemberByUserID(_ context.Context,arg database.GetOrganizationMemberByUserIDParams) (database.OrganizationMember,error) {
2764-
iferr:=validateDatabaseType(arg);err!=nil {
2765-
return database.OrganizationMember{},err
2766-
}
2767-
2768-
q.mutex.RLock()
2769-
deferq.mutex.RUnlock()
2770-
2771-
for_,organizationMember:=rangeq.organizationMembers {
2772-
iforganizationMember.OrganizationID!=arg.OrganizationID {
2773-
continue
2774-
}
2775-
iforganizationMember.UserID!=arg.UserID {
2776-
continue
2777-
}
2778-
returnorganizationMember,nil
2779-
}
2780-
return database.OrganizationMember{},sql.ErrNoRows
2781-
}
2782-
2783-
func (q*FakeQuerier)GetOrganizationMembershipsByUserID(_ context.Context,userID uuid.UUID) ([]database.OrganizationMember,error) {
2784-
q.mutex.RLock()
2785-
deferq.mutex.RUnlock()
2786-
2787-
varmemberships []database.OrganizationMember
2788-
for_,organizationMember:=rangeq.organizationMembers {
2789-
mem:=organizationMember
2790-
ifmem.UserID!=userID {
2791-
continue
2792-
}
2793-
memberships=append(memberships,mem)
2794-
}
2795-
returnmemberships,nil
2796-
}
2797-
27982763
func (q*FakeQuerier)GetOrganizations(_ context.Context) ([]database.Organization,error) {
27992764
q.mutex.RLock()
28002765
deferq.mutex.RUnlock()
@@ -6965,6 +6930,34 @@ func (q *FakeQuerier) ListWorkspaceAgentPortShares(_ context.Context, workspaceI
69656930
returnshares,nil
69666931
}
69676932

6933+
func (q*FakeQuerier)OrganizationMembers(_ context.Context,arg database.OrganizationMembersParams) ([]database.OrganizationMembersRow,error) {
6934+
iferr:=validateDatabaseType(arg);err!=nil {
6935+
return []database.OrganizationMembersRow{},err
6936+
}
6937+
6938+
q.mutex.RLock()
6939+
deferq.mutex.RUnlock()
6940+
6941+
tmp:=make([]database.OrganizationMembersRow,0)
6942+
for_,organizationMember:=rangeq.organizationMembers {
6943+
ifarg.OrganizationID!=uuid.Nil&&organizationMember.OrganizationID!=arg.OrganizationID {
6944+
continue
6945+
}
6946+
6947+
ifarg.UserID!=uuid.Nil&&organizationMember.UserID!=arg.UserID {
6948+
continue
6949+
}
6950+
6951+
organizationMember:=organizationMember
6952+
user,_:=q.getUserByIDNoLock(organizationMember.UserID)
6953+
tmp=append(tmp, database.OrganizationMembersRow{
6954+
OrganizationMember:organizationMember,
6955+
Username:user.Username,
6956+
})
6957+
}
6958+
returntmp,nil
6959+
}
6960+
69686961
func (q*FakeQuerier)ReduceWorkspaceAgentShareLevelToAuthenticatedByTemplate(_ context.Context,templateID uuid.UUID)error {
69696962
err:=validateDatabaseType(templateID)
69706963
iferr!=nil {

‎coderd/database/dbmetrics/dbmetrics.go

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

‎coderd/database/dbmock/dbmock.go

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

0 commit comments

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