Skip to content

Commit b06f943

Browse files
hugodutkahugodutka
committed
revert "simplify RBAC check on group member count" - new query didn't work on empty groups
1 parent d3b4d7c commit b06f943

File tree

10 files changed

+42
-55
lines changed

10 files changed

+42
-55
lines changed

‎coderd/database/dbauthz/dbauthz.go

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1400,8 +1400,22 @@ 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) (database.GetGroupMembersCountByGroupIDRow, error) {
1404-
return fetch(q.log, q.auth, q.db.GetGroupMembersCountByGroupID)(ctx, groupID)
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
14051419
}
14061420

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

‎coderd/database/dbmem/dbmem.go

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2536,20 +2536,12 @@ 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) (database.GetGroupMembersCountByGroupIDRow, error) {
2540-
members, err := q.GetGroupMembersByGroupID(ctx, groupID)
2539+
func (q *FakeQuerier) GetGroupMembersCountByGroupID(ctx context.Context, groupID uuid.UUID) (int64, error) {
2540+
users, err := q.GetGroupMembersByGroupID(ctx, groupID)
25412541
if err != nil {
2542-
return database.GetGroupMembersCountByGroupIDRow{}, err
2543-
}
2544-
group, err := q.GetGroupByID(ctx, groupID)
2545-
if err != nil {
2546-
return database.GetGroupMembersCountByGroupIDRow{}, err
2542+
return 0, err
25472543
}
2548-
return database.GetGroupMembersCountByGroupIDRow{
2549-
OrganizationID: group.OrganizationID,
2550-
GroupID: groupID,
2551-
MemberCount: int64(len(members)),
2552-
}, nil
2544+
return int64(len(users)), nil
25532545
}
25542546

25552547
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: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,12 @@ func (gm GroupMember) RBACObject() rbac.Object {
191191
return rbac.ResourceGroupMember.WithID(gm.UserID).InOrg(gm.OrganizationID).WithOwner(gm.UserID.String())
192192
}
193193

194-
func (r GetGroupMembersCountByGroupIDRow) RBACObject() rbac.Object {
194+
type GroupMembersCountRBACHelper struct {
195+
GroupID uuid.UUID
196+
OrganizationID uuid.UUID
197+
}
198+
199+
func (r GroupMembersCountRBACHelper) RBACObject() rbac.Object {
195200
return groupRBACObject(r.GroupID, r.OrganizationID)
196201
}
197202

‎coderd/database/querier.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/queries.sql.go

Lines changed: 6 additions & 21 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: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,16 +8,7 @@ SELECT * FROM group_members_expanded WHERE group_id = @group_id;
88
-- Returns the total count of members in a group. Shows the total
99
-- count even if the caller does not have read access to ResourceGroupMember.
1010
-- They only need ResourceGroup read access.
11-
SELECT
12-
gme.organization_id,
13-
gme.group_id,
14-
COUNT(*) as member_count
15-
FROM
16-
group_members_expanded gme
17-
WHERE
18-
gme.group_id = @group_id
19-
GROUP BY
20-
gme.organization_id, gme.group_id;
11+
SELECT COUNT(*) FROM group_members_expanded WHERE group_id = @group_id;
2112

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

‎enterprise/coderd/groups.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -291,7 +291,7 @@ func (api *API) patchGroup(rw http.ResponseWriter, r *http.Request) {
291291
return
292292
}
293293

294-
httpapi.Write(ctx, rw, http.StatusOK, db2sdk.Group(group, patchedMembers, int(memberCount.MemberCount)))
294+
httpapi.Write(ctx, rw, http.StatusOK, db2sdk.Group(group, patchedMembers, int(memberCount)))
295295
}
296296

297297
// @Summary Delete group by name
@@ -382,7 +382,7 @@ func (api *API) group(rw http.ResponseWriter, r *http.Request) {
382382
return
383383
}
384384

385-
httpapi.Write(ctx, rw, http.StatusOK, db2sdk.Group(group, users, int(memberCount.MemberCount)))
385+
httpapi.Write(ctx, rw, http.StatusOK, db2sdk.Group(group, users, int(memberCount)))
386386
}
387387

388388
// @Summary Get groups by organization
@@ -432,7 +432,7 @@ func (api *API) groups(rw http.ResponseWriter, r *http.Request) {
432432
return
433433
}
434434

435-
resp = append(resp, db2sdk.Group(group, members, int(memberCount.MemberCount)))
435+
resp = append(resp, db2sdk.Group(group, members, int(memberCount)))
436436
}
437437

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

‎enterprise/coderd/templates.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ func (api *API) templateAvailablePermissions(rw http.ResponseWriter, r *http.Req
7070
return
7171
}
7272

73-
sdkGroups = append(sdkGroups, db2sdk.Group(group, members, int(memberCount.MemberCount)))
73+
sdkGroups = append(sdkGroups, db2sdk.Group(group, members, int(memberCount)))
7474
}
7575

7676
httpapi.Write(ctx, rw, http.StatusOK, codersdk.ACLAvailable{
@@ -145,7 +145,7 @@ func (api *API) templateACL(rw http.ResponseWriter, r *http.Request) {
145145
return
146146
}
147147
groups = append(groups, codersdk.TemplateGroup{
148-
Group: db2sdk.Group(group.Group, members, int(memberCount.MemberCount)),
148+
Group: db2sdk.Group(group.Group, members, int(memberCount)),
149149
Role: convertToTemplateRole(group.Actions),
150150
})
151151
}

0 commit comments

Comments
 (0)