opensource/coder
3
0
Fork 0
mirror of https://github.com/coder/coder.git synced 2026-09-24 15:04:27 +08:00
Code Issues Packages Projects Releases Wiki Activity

fix: only return group member count for workspace acl (#26206)

Browse Source
This commit is contained in:
Jon Ayers
2026-06-23 11:58:00 -05:00
committed by GitHub
parent 6acf32701e
commit c7ddcce62c
5 changed files with 150 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files
coderd/workspaces.go
+32 -26
View File
@@ -2300,13 +2300,13 @@ func (api *API) workspaceACL(rw http.ResponseWriter, r *http.Request) {
return
}
// This is largely based on the template ACL implementation, and is far from
// ideal. Usually, when we use the System context it's because we need to
// run some query that won't actually be exposed to the user. That is not
// the case here. This data goes directly to an unauthorized user. We are
// just straight up breaking security promises.
//
// TODO: This needs to be fixed before GA. Currently in beta.
// Callers are authorized to read this workspace, not necessarily the
// users and groups on its ACL. We deliberately use the System context to
// look up that data, but only return minimal identity information that is
// safe to expose to anyone who can read the ACL: MinimalUser for ACL users
// (no email or other PII) and group identity plus a member count for ACL
// groups (no member roster). This mirrors the chat ACL and template
// available-ACL endpoints.
// Fetch all of the users and their organization memberships
userIDs := make([]uuid.UUID, 0, len(workspaceACL.Users))
@@ -2318,7 +2318,8 @@ func (api *API) workspaceACL(rw http.ResponseWriter, r *http.Request) {
}
userIDs = append(userIDs, id)
}
// For context see https://github.com/coder/coder/pull/19375
// ACL users are returned as MinimalUser, which contains no PII, so it is
// safe to fetch them under the System context.
// nolint:gocritic
dbUsers, err := api.Database.GetUsersByIDs(dbauthz.AsSystemRestricted(ctx), userIDs)
if err != nil && !xerrors.Is(err, sql.ErrNoRows) {
@@ -2350,7 +2351,8 @@ func (api *API) workspaceACL(rw http.ResponseWriter, r *http.Request) {
// before making the DB call.
dbGroups := make([]database.GetGroupsRow, 0)
if len(groupIDs) > 0 {
// For context see https://github.com/coder/coder/pull/19375
// Group identity must be visible to anyone who can read the ACL so
// that owners and shared users can see and manage entries.
// nolint:gocritic
dbGroups, err = api.Database.GetGroups(dbauthz.AsSystemRestricted(ctx), database.GetGroupsParams{GroupIds: groupIDs})
if err != nil && !xerrors.Is(err, sql.ErrNoRows) {
@@ -2359,26 +2361,30 @@ func (api *API) workspaceACL(rw http.ResponseWriter, r *http.Request) {
}
}
// Fetch member counts for all groups in a single query to avoid an N+1
// lookup. We intentionally do not populate the per-group member rosters:
// callers authorized to read the ACL are not necessarily authorized to
// read group membership, and the roster includes member PII. Only the
// total member count is returned (see Group.TotalMemberCount).
// nolint:gocritic
countRows, err := api.Database.GetGroupMembersCountByGroupIDs(dbauthz.AsSystemRestricted(ctx), database.GetGroupMembersCountByGroupIDsParams{
GroupIds: groupIDs,
IncludeSystem: false,
})
if err != nil && !xerrors.Is(err, sql.ErrNoRows) {
httpapi.InternalServerError(rw, err)
return
}
countByGroup := make(map[uuid.UUID]int64, len(countRows))
for _, row := range countRows {
countByGroup[row.GroupID] = row.MemberCount
}
groups := make([]codersdk.WorkspaceGroup, 0, len(dbGroups))
for _, it := range dbGroups {
var members []database.GroupMember
// For context see https://github.com/coder/coder/pull/19375
// nolint:gocritic
members, err = api.Database.GetGroupMembersByGroupID(dbauthz.AsSystemRestricted(ctx), database.GetGroupMembersByGroupIDParams{
GroupID: it.Group.ID,
IncludeSystem: false,
})
if err != nil {
httpapi.InternalServerError(rw, err)
return
}
groups = append(groups, codersdk.WorkspaceGroup{
Group: db2sdk.Group(database.GetGroupsRow{
Group: it.Group,
OrganizationName: it.OrganizationName,
OrganizationDisplayName: it.OrganizationDisplayName,
}, members, len(members)),
Role: convertToWorkspaceRole(workspaceACL.Groups[it.Group.ID.String()].Permissions),
Group: db2sdk.Group(it, nil, int(countByGroup[it.Group.ID])),
Role: convertToWorkspaceRole(workspaceACL.Groups[it.Group.ID.String()].Permissions),
})
}