Skip to content

Commit de06b53

Browse files
sreyasreya
committed
feat: allow setting quotas on Everyone group
1 parent 5b2ea2e commit de06b53

File tree

13 files changed

+135
-49
lines changed

13 files changed

+135
-49
lines changed

‎coderd/database/dbauthz/dbauthz.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -916,11 +916,11 @@ func (q *querier) GetGroupByOrgAndName(ctx context.Context, arg database.GetGrou
916916
return fetch(q.log, q.auth, q.db.GetGroupByOrgAndName)(ctx, arg)
917917
}
918918

919-
func (q *querier) GetGroupMembers(ctx context.Context, groupID uuid.UUID) ([]database.User, error) {
920-
if _, err := q.GetGroupByID(ctx, groupID); err != nil { // AuthZ check
919+
func (q *querier) GetGroupMembers(ctx context.Context, arg database.GetGroupMembersParams) ([]database.User, error) {
920+
if _, err := q.GetGroupByID(ctx, arg.ID); err != nil { // AuthZ check
921921
return nil, err
922922
}
923-
return q.db.GetGroupMembers(ctx, groupID)
923+
return q.db.GetGroupMembers(ctx, arg)
924924
}
925925

926926
func (q *querier) GetGroupsByOrganizationID(ctx context.Context, organizationID uuid.UUID) ([]database.Group, error) {

‎coderd/database/dbfake/dbfake.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -1376,13 +1376,13 @@ func (q *FakeQuerier) GetGroupByOrgAndName(_ context.Context, arg database.GetGr
13761376
return database.Group{}, sql.ErrNoRows
13771377
}
13781378

1379-
func (q *FakeQuerier) GetGroupMembers(_ context.Context, groupID uuid.UUID) ([]database.User, error) {
1379+
func (q *FakeQuerier) GetGroupMembers(_ context.Context, arg database.GetGroupMembersParams) ([]database.User, error) {
13801380
q.mutex.RLock()
13811381
defer q.mutex.RUnlock()
13821382

13831383
var members []database.GroupMember
13841384
for _, member := range q.groupMembers {
1385-
if member.GroupID == groupID {
1385+
if member.GroupID == arg.ID {
13861386
members = append(members, member)
13871387
}
13881388
}

‎coderd/database/dbmetrics/dbmetrics.go

+1-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/dbmock/dbmock.go

+1-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/modelmethods.go

+4
Original file line numberDiff line numberDiff line change
@@ -362,3 +362,7 @@ func ConvertWorkspaceRows(rows []GetWorkspacesRow) []Workspace {
362362

363363
return workspaces
364364
}
365+
366+
func (g Group) IsEveryoneGroup() bool {
367+
return g.ID == g.OrganizationID
368+
}

‎coderd/database/querier.go

+1-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries.sql.go

+24-7
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries/groupmembers.sql

+17-3
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,26 @@ SELECT
33
users.*
44
FROM
55
users
6-
JOIN
6+
LEFT JOIN
77
group_members
88
ON
9-
users.id = group_members.user_id
9+
CASE WHEN @id:: uuid != @organization_id :: uuid THEN
10+
group_members.user_id = users.id
11+
END
12+
LEFT JOIN
13+
organization_members
14+
ON
15+
CASE WHEN @id :: uuid = @organization_id :: uuid THEN
16+
organization_members.user_id = users.id
17+
END
1018
WHERE
11-
group_members.group_id = $1
19+
CASE WHEN @id :: uuid != @organization_id :: uuid THEN
20+
group_members.group_id = @id
21+
ELSE true END
22+
AND
23+
CASE WHEN @id :: uuid = @organization_id :: uuid THEN
24+
organization_members.organization_id = @organization_id
25+
ELSE true END
1226
AND
1327
users.status = 'active'
1428
AND

‎coderd/database/queries/groups.sql

+1-3
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,7 @@ SELECT
2626
FROM
2727
groups
2828
WHERE
29-
organization_id = $1
30-
AND
31-
id != $1;
29+
organization_id = $1;
3230

3331
-- name: InsertGroup :one
3432
INSERT INTO groups (

‎enterprise/coderd/groups.go

+60-24
Original file line numberDiff line numberDiff line change
@@ -102,16 +102,26 @@ func (api *API) patchGroup(rw http.ResponseWriter, r *http.Request) {
102102
)
103103
defer commitAudit()
104104

105-
currentMembers, currentMembersErr := api.Database.GetGroupMembers(ctx, group.ID)
106-
if currentMembersErr != nil {
107-
httpapi.InternalServerError(rw, currentMembersErr)
105+
var req codersdk.PatchGroupRequest
106+
if !httpapi.Read(ctx, rw, r, &req) {
108107
return
109108
}
110109

111-
aReq.Old = group.Auditable(currentMembers)
110+
// If the name matches the existing group name pretend we aren't
111+
// updating the name at all.
112+
if req.Name == group.Name {
113+
req.Name = ""
114+
}
112115

113-
var req codersdk.PatchGroupRequest
114-
if !httpapi.Read(ctx, rw, r, &req) {
116+
// TODO:
117+
// - Test no add/remove users to everyone group.
118+
// - Test no update everyone group name.
119+
// - Test no update everyone display name.
120+
121+
if group.ID == group.OrganizationID && (req.Name != "" || req.DisplayName != nil) {
122+
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
123+
Message: fmt.Sprintf("Cannot rename the %q group!", database.AllUsersGroup),
124+
})
115125
return
116126
}
117127

@@ -122,16 +132,27 @@ func (api *API) patchGroup(rw http.ResponseWriter, r *http.Request) {
122132
return
123133
}
124134

125-
// If the name matches the existing group name pretend we aren't
126-
// updating the name at all.
127-
if req.Name == group.Name {
128-
req.Name = ""
129-
}
130-
131135
users := make([]string, 0, len(req.AddUsers)+len(req.RemoveUsers))
132136
users = append(users, req.AddUsers...)
133137
users = append(users, req.RemoveUsers...)
134138

139+
if len(users) > 0 && group.Name == database.AllUsersGroup {
140+
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
141+
Message: fmt.Sprintf("Cannot add or remove users from the %q group!", database.AllUsersGroup),
142+
})
143+
return
144+
}
145+
146+
currentMembers, currentMembersErr := api.Database.GetGroupMembers(ctx, database.GetGroupMembersParams{
147+
ID: group.ID,
148+
OrganizationID: group.OrganizationID,
149+
})
150+
if currentMembersErr != nil {
151+
httpapi.InternalServerError(rw, currentMembersErr)
152+
return
153+
}
154+
aReq.Old = group.Auditable(currentMembers)
155+
135156
for _, id := range users {
136157
if _, err := uuid.Parse(id); err != nil {
137158
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
@@ -156,6 +177,7 @@ func (api *API) patchGroup(rw http.ResponseWriter, r *http.Request) {
156177
return
157178
}
158179
}
180+
159181
if req.Name != "" && req.Name != group.Name {
160182
_, err := api.Database.GetGroupByOrgAndName(ctx, database.GetGroupByOrgAndNameParams{
161183
OrganizationID: group.OrganizationID,
@@ -230,7 +252,9 @@ func (api *API) patchGroup(rw http.ResponseWriter, r *http.Request) {
230252
}
231253
}
232254
return nil
233-
}, nil)
255+
}, &sql.TxOptions{
256+
Isolation: sql.LevelRepeatableRead,
257+
})
234258
if database.IsUniqueViolation(err) {
235259
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
236260
Message: "Cannot add the same user to a group twice!",
@@ -250,7 +274,10 @@ func (api *API) patchGroup(rw http.ResponseWriter, r *http.Request) {
250274
return
251275
}
252276

253-
patchedMembers, err := api.Database.GetGroupMembers(ctx, group.ID)
277+
patchedMembers, err := api.Database.GetGroupMembers(ctx, database.GetGroupMembersParams{
278+
ID: group.ID,
279+
OrganizationID: group.OrganizationID,
280+
})
254281
if err != nil {
255282
httpapi.InternalServerError(rw, err)
256283
return
@@ -283,21 +310,24 @@ func (api *API) deleteGroup(rw http.ResponseWriter, r *http.Request) {
283310
)
284311
defer commitAudit()
285312

286-
groupMembers, getMembersErr := api.Database.GetGroupMembers(ctx, group.ID)
287-
if getMembersErr != nil {
288-
httpapi.InternalServerError(rw, getMembersErr)
289-
return
290-
}
291-
292-
aReq.Old = group.Auditable(groupMembers)
293-
294313
if group.Name == database.AllUsersGroup {
295314
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
296315
Message: fmt.Sprintf("%q is a reserved group and cannot be deleted!", database.AllUsersGroup),
297316
})
298317
return
299318
}
300319

320+
groupMembers, getMembersErr := api.Database.GetGroupMembers(ctx, database.GetGroupMembersParams{
321+
ID: group.ID,
322+
OrganizationID: group.OrganizationID,
323+
})
324+
if getMembersErr != nil {
325+
httpapi.InternalServerError(rw, getMembersErr)
326+
return
327+
}
328+
329+
aReq.Old = group.Auditable(groupMembers)
330+
301331
err := api.Database.DeleteGroupByID(ctx, group.ID)
302332
if err != nil {
303333
httpapi.InternalServerError(rw, err)
@@ -336,7 +366,10 @@ func (api *API) group(rw http.ResponseWriter, r *http.Request) {
336366
group = httpmw.GroupParam(r)
337367
)
338368

339-
users, err := api.Database.GetGroupMembers(ctx, group.ID)
369+
users, err := api.Database.GetGroupMembers(ctx, database.GetGroupMembersParams{
370+
ID: group.ID,
371+
OrganizationID: group.OrganizationID,
372+
})
340373
if err != nil && !xerrors.Is(err, sql.ErrNoRows) {
341374
httpapi.InternalServerError(rw, err)
342375
return
@@ -381,7 +414,10 @@ func (api *API) groups(rw http.ResponseWriter, r *http.Request) {
381414

382415
resp := make([]codersdk.Group, 0, len(groups))
383416
for _, group := range groups {
384-
members, err := api.Database.GetGroupMembers(ctx, group.ID)
417+
members, err := api.Database.GetGroupMembers(ctx, database.GetGroupMembersParams{
418+
ID: group.ID,
419+
OrganizationID: group.OrganizationID,
420+
})
385421
if err != nil {
386422
httpapi.InternalServerError(rw, err)
387423
return

‎enterprise/coderd/templates.go

+8-2
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,10 @@ func (api *API) templateAvailablePermissions(rw http.ResponseWriter, r *http.Req
5858
sdkGroups := make([]codersdk.Group, 0, len(groups))
5959
for _, group := range groups {
6060
// nolint:gocritic
61-
members, err := api.Database.GetGroupMembers(dbauthz.AsSystemRestricted(ctx), group.ID)
61+
members, err := api.Database.GetGroupMembers(dbauthz.AsSystemRestricted(ctx), database.GetGroupMembersParams{
62+
ID: group.ID,
63+
OrganizationID: group.OrganizationID,
64+
})
6265
if err != nil {
6366
httpapi.InternalServerError(rw, err)
6467
return
@@ -128,7 +131,10 @@ func (api *API) templateACL(rw http.ResponseWriter, r *http.Request) {
128131
// them read the group members.
129132
// We should probably at least return more truncated user data here.
130133
// nolint:gocritic
131-
members, err = api.Database.GetGroupMembers(dbauthz.AsSystemRestricted(ctx), group.ID)
134+
members, err = api.Database.GetGroupMembers(dbauthz.AsSystemRestricted(ctx), database.GetGroupMembersParams{
135+
ID: group.ID,
136+
OrganizationID: group.OrganizationID,
137+
})
132138
if err != nil {
133139
httpapi.InternalServerError(rw, err)
134140
return

‎site/src/pages/GroupsPage/GroupPage.tsx

+10-2
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,7 @@ export const GroupPage: React.FC = () => {
124124
<Button startIcon={<SettingsOutlined />}>Settings</Button>
125125
</Link>
126126
<Button
127+
disabled={group?.id == group?.organization_id}
127128
onClick={() => {
128129
send("DELETE")
129130
}}
@@ -146,7 +147,13 @@ export const GroupPage: React.FC = () => {
146147
</PageHeader>
147148

148149
<Stack spacing={1}>
149-
<Maybe condition={canUpdateGroup}>
150+
<Maybe
151+
condition={
152+
canUpdateGroup &&
153+
group !== undefined &&
154+
group?.id != group?.organization_id
155+
}
156+
>
150157
<AddGroupMember
151158
isLoading={state.matches("addingMember")}
152159
onSubmit={(user, reset) => {
@@ -217,7 +224,8 @@ export const GroupPage: React.FC = () => {
217224
userId: member.id,
218225
})
219226
},
220-
disabled: false,
227+
disabled:
228+
group.id == group.organization_id,
221229
},
222230
]}
223231
/>

‎site/src/pages/GroupsPage/SettingsGroupPageView.tsx

+3
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import { useTranslation } from "react-i18next"
1212
import { getFormHelpers, nameValidator, onChangeTrimmed } from "utils/formUtils"
1313
import * as Yup from "yup"
1414
import { Stack } from "components/Stack/Stack"
15+
import { GroupRemove } from "@mui/icons-material"
1516

1617
type FormData = {
1718
name: string
@@ -56,6 +57,7 @@ const UpdateGroupForm: FC<{
5657
autoFocus
5758
fullWidth
5859
label="Name"
60+
disabled={group.id == group.organization_id}
5961
/>
6062
<TextField
6163
{...getFieldHelpers(
@@ -67,6 +69,7 @@ const UpdateGroupForm: FC<{
6769
autoFocus
6870
fullWidth
6971
label="Display Name"
72+
disabled={group.id == group.organization_id}
7073
/>
7174
<LazyIconField
7275
{...getFieldHelpers("avatar_url")}

0 commit comments

Comments
 (0)