Skip to content

Commit e5c5f3f

Browse files
committed
simplify RBAC check on group member count
1 parent 0aea7f8 commit e5c5f3f

File tree

9 files changed

+63
-49
lines changed

9 files changed

+63
-49
lines changed

coderd/database/dbauthz/dbauthz.go

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1400,22 +1400,8 @@ func (q *querier) GetGroupMembersByGroupID(ctx context.Context, id uuid.UUID) ([
14001400
return fetchWithPostFilter(q.auth, policy.ActionRead, q.db.GetGroupMembersByGroupID)(ctx, id)
14011401
}
14021402

1403-
func (q *querier) GetGroupMembersCountByGroupID(ctx context.Context, groupID uuid.UUID) (int64, error) {
1404-
group, err := q.GetGroupByID(ctx, groupID)
1405-
if err != nil {
1406-
return 0, err
1407-
}
1408-
memberCount, err := q.db.GetGroupMembersCountByGroupID(ctx, groupID)
1409-
if err != nil {
1410-
return 0, err
1411-
}
1412-
if err := q.authorizeContext(ctx, policy.ActionRead, database.GroupMembersCountRBACHelper{
1413-
GroupID: groupID,
1414-
OrganizationID: group.OrganizationID,
1415-
}); err != nil {
1416-
return 0, err
1417-
}
1418-
return memberCount, nil
1403+
func (q *querier) GetGroupMembersCountByGroupID(ctx context.Context, groupID uuid.UUID) (database.GetGroupMembersCountByGroupIDRow, error) {
1404+
return fetch(q.log, q.auth, q.db.GetGroupMembersCountByGroupID)(ctx, groupID)
14191405
}
14201406

14211407
func (q *querier) GetGroups(ctx context.Context) ([]database.Group, error) {

coderd/database/dbmem/dbmem.go

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2536,12 +2536,20 @@ func (q *FakeQuerier) GetGroupMembersByGroupID(_ context.Context, id uuid.UUID)
25362536
return members, nil
25372537
}
25382538

2539-
func (q *FakeQuerier) GetGroupMembersCountByGroupID(ctx context.Context, groupID uuid.UUID) (int64, error) {
2540-
users, err := q.GetGroupMembersByGroupID(ctx, groupID)
2539+
func (q *FakeQuerier) GetGroupMembersCountByGroupID(ctx context.Context, groupID uuid.UUID) (database.GetGroupMembersCountByGroupIDRow, error) {
2540+
members, err := q.GetGroupMembersByGroupID(ctx, groupID)
25412541
if err != nil {
2542-
return 0, err
2542+
return database.GetGroupMembersCountByGroupIDRow{}, err
2543+
}
2544+
group, err := q.GetGroupByID(ctx, groupID)
2545+
if err != nil {
2546+
return database.GetGroupMembersCountByGroupIDRow{}, err
25432547
}
2544-
return int64(len(users)), nil
2548+
return database.GetGroupMembersCountByGroupIDRow{
2549+
OrganizationID: group.OrganizationID,
2550+
GroupID: groupID,
2551+
MemberCount: int64(len(members)),
2552+
}, nil
25452553
}
25462554

25472555
func (q *FakeQuerier) GetGroups(_ context.Context) ([]database.Group, error) {

coderd/database/dbmetrics/dbmetrics.go

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

coderd/database/dbmock/dbmock.go

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

coderd/database/modelmethods.go

Lines changed: 10 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -172,34 +172,27 @@ func (v TemplateVersion) RBACObjectNoTemplate() rbac.Object {
172172
return rbac.ResourceTemplate.InOrg(v.OrganizationID)
173173
}
174174

175-
func (g Group) RBACObject() rbac.Object {
176-
return rbac.ResourceGroup.WithID(g.ID).
177-
InOrg(g.OrganizationID).
175+
func groupRBACObject(groupID, organizationID uuid.UUID) rbac.Object {
176+
return rbac.ResourceGroup.WithID(groupID).
177+
InOrg(organizationID).
178178
// Group members can read the group.
179179
WithGroupACL(map[string][]policy.Action{
180-
g.ID.String(): {
180+
groupID.String(): {
181181
policy.ActionRead,
182182
},
183183
})
184184
}
185185

186-
func (gm GroupMember) RBACObject() rbac.Object {
187-
return rbac.ResourceGroupMember.WithID(gm.UserID).InOrg(gm.OrganizationID).WithOwner(gm.UserID.String())
186+
func (g Group) RBACObject() rbac.Object {
187+
return groupRBACObject(g.ID, g.OrganizationID)
188188
}
189189

190-
type GroupMembersCountRBACHelper struct {
191-
GroupID uuid.UUID
192-
OrganizationID uuid.UUID
190+
func (gm GroupMember) RBACObject() rbac.Object {
191+
return rbac.ResourceGroupMember.WithID(gm.UserID).InOrg(gm.OrganizationID).WithOwner(gm.UserID.String())
193192
}
194193

195-
func (r GroupMembersCountRBACHelper) RBACObject() rbac.Object {
196-
return rbac.ResourceGroup.WithID(r.GroupID).InOrg(r.OrganizationID).
197-
// Group members can read the member count.
198-
WithGroupACL(map[string][]policy.Action{
199-
r.GroupID.String(): {
200-
policy.ActionRead,
201-
},
202-
})
194+
func (r GetGroupMembersCountByGroupIDRow) RBACObject() rbac.Object {
195+
return groupRBACObject(r.GroupID, r.OrganizationID)
203196
}
204197

205198
func (w GetWorkspaceByAgentIDRow) RBACObject() rbac.Object {

coderd/database/querier.go

Lines changed: 2 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

coderd/database/queries.sql.go

Lines changed: 22 additions & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

coderd/database/queries/groupmembers.sql

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,17 @@ SELECT * FROM group_members_expanded;
55
SELECT * FROM group_members_expanded WHERE group_id = @group_id;
66

77
-- name: GetGroupMembersCountByGroupID :one
8-
SELECT COUNT(*) FROM group_members_expanded WHERE group_id = @group_id;
8+
SELECT
9+
gme.organization_id,
10+
gme.group_id,
11+
COUNT(*) as member_count
12+
FROM
13+
group_members_expanded gme
14+
WHERE
15+
gme.group_id = @group_id
16+
-- This aggregation is guaranteed to return a single row
17+
GROUP BY
18+
gme.organization_id, gme.group_id;
919

1020
-- InsertUserGroupsByName adds a user to all provided groups, if they exist.
1121
-- name: InsertUserGroupsByName :exec

enterprise/coderd/groups.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -453,7 +453,7 @@ func (api *API) reducedGroupsByUserAndOrganization(rw http.ResponseWriter, r *ht
453453
return
454454
}
455455

456-
resp = append(resp, db2sdk.ReducedGroup(group, int(memberCount)))
456+
resp = append(resp, db2sdk.ReducedGroup(group, int(memberCount.MemberCount)))
457457
}
458458

459459
httpapi.Write(ctx, rw, http.StatusOK, resp)

0 commit comments

Comments
 (0)