Skip to content

Commit 475f6e2

Browse files
code-ashercode-asher
committed
Limit the number of org permission checks
We show the org on the sidebar if they can edit anything, and we show each sub-link if they can view it, which means we were making both edit and view permission checks. Instead, show each link if they can edit it (not just view), which negates the need for separate view permissions. Incidentally, this also reduces the number of checks we need to make for individual pages, since some of them were only used on the sidebar.
1 parent 9f39257 commit 475f6e2

File tree

4 files changed

+83
-80
lines changed

4 files changed

+83
-80
lines changed

‎site/src/api/queries/organizations.ts

+63-59
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import type { QueryClient } from "react-query";
22
import { API } from "api/api";
33
import type {
4-
AuthorizationCheck,
54
AuthorizationResponse,
65
CreateOrganizationRequest,
76
Organization,
@@ -124,60 +123,6 @@ export const provisionerDaemons = (organization: string) => {
124123
};
125124
};
126125

127-
const orgChecks = (
128-
organizationId: string,
129-
): Record<string, AuthorizationCheck> => ({
130-
viewMembers: {
131-
object: {
132-
resource_type: "organization_member",
133-
organization_id: organizationId,
134-
},
135-
action: "read",
136-
},
137-
editMembers: {
138-
object: {
139-
resource_type: "organization_member",
140-
organization_id: organizationId,
141-
},
142-
action: "update",
143-
},
144-
createGroup: {
145-
object: {
146-
resource_type: "group",
147-
organization_id: organizationId,
148-
},
149-
action: "create",
150-
},
151-
viewGroups: {
152-
object: {
153-
resource_type: "group",
154-
organization_id: organizationId,
155-
},
156-
action: "read",
157-
},
158-
editGroups: {
159-
object: {
160-
resource_type: "group",
161-
organization_id: organizationId,
162-
},
163-
action: "update",
164-
},
165-
editOrganization: {
166-
object: {
167-
resource_type: "organization",
168-
organization_id: organizationId,
169-
},
170-
action: "update",
171-
},
172-
auditOrganization: {
173-
object: {
174-
resource_type: "audit_log",
175-
organization_id: organizationId,
176-
},
177-
action: "read",
178-
},
179-
});
180-
181126
/**
182127
* Fetch permissions for a single organization.
183128
*
@@ -190,7 +135,31 @@ export const organizationPermissions = (organizationId: string | undefined) => {
190135
return {
191136
queryKey: ["organization", organizationId, "permissions"],
192137
queryFn: () =>
193-
API.checkAuthorization({ checks: orgChecks(organizationId) }),
138+
// Only request what we use on individual org settings, members, and group
139+
// pages, which at the moment is whether you can edit the members on the
140+
// members page and whether you can see the create group button on the
141+
// groups page. The edit organization check for the settings page is
142+
// covered by the multi-org query at the moment, and the edit group check
143+
// on the group page is done on the group itself, not the org, so neither
144+
// show up here.
145+
API.checkAuthorization({
146+
checks: {
147+
editMembers: {
148+
object: {
149+
resource_type: "organization_member",
150+
organization_id: organizationId,
151+
},
152+
action: "update",
153+
},
154+
createGroup: {
155+
object: {
156+
resource_type: "group",
157+
organization_id: organizationId,
158+
},
159+
action: "create",
160+
},
161+
},
162+
}),
194163
};
195164
};
196165

@@ -209,19 +178,54 @@ export const organizationsPermissions = (
209178
return {
210179
queryKey: ["organizations", "permissions"],
211180
queryFn: async () => {
181+
// Only request what we need for the sidebar, which is one edit permission
182+
// per sub-link (audit page, settings page, groups page, and members page)
183+
// that tells us whether to show that page, since we only show them if you
184+
// can edit (and not, at the moment if you can only view).
185+
const checks = (organizationId: string) => ({
186+
editMembers: {
187+
object: {
188+
resource_type: "organization_member",
189+
organization_id: organizationId,
190+
},
191+
action: "update",
192+
},
193+
editGroups: {
194+
object: {
195+
resource_type: "group",
196+
organization_id: organizationId,
197+
},
198+
action: "update",
199+
},
200+
editOrganization: {
201+
object: {
202+
resource_type: "organization",
203+
organization_id: organizationId,
204+
},
205+
action: "update",
206+
},
207+
auditOrganization: {
208+
object: {
209+
resource_type: "audit_log",
210+
organization_id: organizationId,
211+
},
212+
action: "read",
213+
},
214+
});
215+
212216
// The endpoint takes a flat array, so to avoid collisions prepend each
213217
// check with the org ID (the key can be anything we want).
214-
const checks = organizations
218+
const prefixedChecks = organizations
215219
.map((org) =>
216-
Object.entries(orgChecks(org.id)).map(([key, val]) => [
220+
Object.entries(checks(org.id)).map(([key, val]) => [
217221
`${org.id}.${key}`,
218222
val,
219223
]),
220224
)
221225
.flat();
222226

223227
const response = await API.checkAuthorization({
224-
checks: Object.fromEntries(checks),
228+
checks: Object.fromEntries(prefixedChecks),
225229
});
226230

227231
// Now we can unflatten by parsing out the org ID from each check.

‎site/src/pages/ManagementSettingsPage/OrganizationMembersPage.test.tsx

-1
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ beforeEach(() => {
2828
http.post("/api/v2/authcheck", async () => {
2929
return HttpResponse.json({
3030
editMembers: true,
31-
viewMembers: true,
3231
viewDeploymentValues: true,
3332
});
3433
}),

‎site/src/pages/ManagementSettingsPage/SidebarView.stories.tsx

+18-18
Original file line numberDiff line numberDiff line change
@@ -17,17 +17,17 @@ const meta: Meta<typeof SidebarView> = {
1717
MockOrganization,
1818
{
1919
editOrganization: true,
20-
viewMembers: true,
21-
viewGroups: true,
20+
editMembers: true,
21+
editGroups: true,
2222
auditOrganization: true,
2323
},
2424
],
2525
[
2626
MockOrganization2,
2727
{
2828
editOrganization: true,
29-
viewMembers: true,
30-
viewGroups: true,
29+
editMembers: true,
30+
editGroups: true,
3131
auditOrganization: true,
3232
},
3333
],
@@ -118,8 +118,8 @@ export const SelectedOrgAdmin: Story = {
118118
MockOrganization,
119119
{
120120
editOrganization: true,
121-
viewMembers: true,
122-
viewGroups: true,
121+
editMembers: true,
122+
editGroups: true,
123123
auditOrganization: true,
124124
},
125125
],
@@ -139,8 +139,8 @@ export const SelectedOrgAuditor: Story = {
139139
MockOrganization,
140140
{
141141
editOrganization: false,
142-
viewMembers: false,
143-
viewGroups: false,
142+
editMembers: false,
143+
editGroups: false,
144144
auditOrganization: true,
145145
},
146146
],
@@ -160,8 +160,8 @@ export const SelectedOrgUserAdmin: Story = {
160160
MockOrganization,
161161
{
162162
editOrganization: false,
163-
viewMembers: true,
164-
viewGroups: true,
163+
editMembers: true,
164+
editGroups: true,
165165
auditOrganization: false,
166166
},
167167
],
@@ -176,17 +176,17 @@ export const MultiOrgAdminAndUserAdmin: Story = {
176176
MockOrganization,
177177
{
178178
editOrganization: false,
179-
viewMembers: false,
180-
viewGroups: false,
179+
editMembers: false,
180+
editGroups: false,
181181
auditOrganization: true,
182182
},
183183
],
184184
[
185185
MockOrganization2,
186186
{
187187
editOrganization: false,
188-
viewMembers: true,
189-
viewGroups: true,
188+
editMembers: true,
189+
editGroups: true,
190190
auditOrganization: false,
191191
},
192192
],
@@ -202,17 +202,17 @@ export const SelectedMultiOrgAdminAndUserAdmin: Story = {
202202
MockOrganization,
203203
{
204204
editOrganization: false,
205-
viewMembers: false,
206-
viewGroups: false,
205+
editMembers: false,
206+
editGroups: false,
207207
auditOrganization: true,
208208
},
209209
],
210210
[
211211
MockOrganization2,
212212
{
213213
editOrganization: false,
214-
viewMembers: true,
215-
viewGroups: true,
214+
editMembers: true,
215+
editGroups: true,
216216
auditOrganization: false,
217217
},
218218
],

‎site/src/pages/ManagementSettingsPage/SidebarView.tsx

+2-2
Original file line numberDiff line numberDiff line change
@@ -234,14 +234,14 @@ const OrganizationSettingsNavigation: FC<
234234
Organization settings
235235
</SidebarNavSubItem>
236236
)}
237-
{props.permissions.viewMembers && (
237+
{props.permissions.editMembers && (
238238
<SidebarNavSubItem
239239
href={urlForSubpage(props.organization.name, "members")}
240240
>
241241
Members
242242
</SidebarNavSubItem>
243243
)}
244-
{props.permissions.viewGroups && (
244+
{props.permissions.editGroups && (
245245
<SidebarNavSubItem
246246
href={urlForSubpage(props.organization.name, "groups")}
247247
>

0 commit comments

Comments
 (0)