8000 Limit the number of org permission checks · coder/coder@92d15c5 · GitHub
[go: up one dir, main page]
Skip to content

Commit 92d15c5

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. I also removed the per-org audit link since we are going to rely on the main audit page, so that brings us to three checks per organization on the sidebar. 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 92d15c5

File tree

4 files changed

+77
-103
lines changed

4 files changed

+77
-103
lines changed

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

Lines changed: 56 additions & 59 deletions
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,47 @@ 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 (settings page, groups page, and members page) that tells
183+
// us whether to show that page, since we only show them if you can edit
184+
// (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+
});
208+
212209
// The endpoint takes a flat array, so to avoid collisions prepend each
213210
// check with the org ID (the key can be anything we want).
214-
const checks = organizations
211+
const prefixedChecks = organizations
215212
.map((org) =>
216-
Object.entries(orgChecks(org.id)).map(([key, val]) => [
213+
Object.entries(checks(org.id)).map(([key, val]) => [
217214
`${org.id}.${key}`,
218215
val,
219216
]),
220217
)
221218
.flat();
222219

223220
const response = await API.checkAuthorization({
224-
checks: Object.fromEntries(checks),
221+
checks: Object.fromEntries(prefixedChecks),
225222
});
226223

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

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

Lines changed: 0 additions & 1 deletion
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

Lines changed: 18 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -17,18 +17,16 @@ const meta: Meta<typeof SidebarView> = {
1717
MockOrganization,
1818
{
1919
editOrganization: true,
20-
viewMembers: true,
21-
viewGroups: true,
22-
auditOrganization: true,
20+
editMembers: true,
21+
editGroups: true,
2322
},
2423
],
2524
[
2625
MockOrganization2,
2726
{
2827
editOrganization: true,
29-
viewMembers: true,
30-
viewGroups: true,
31-
auditOrganization: true,
28+
editMembers: true,
29+
editGroups: true,
3230
},
3331
],
3432
],
@@ -118,9 +116,8 @@ export const SelectedOrgAdmin: Story = {
118116
MockOrganization,
119117
{
120118
editOrganization: true,
121-
viewMembers: true,
122-
viewGroups: true,
123-
auditOrganization: true,
119+
editMembers: true,
120+
editGroups: true,
124121
},
125122
],
126123
],
@@ -139,9 +136,8 @@ export const SelectedOrgAuditor: Story = {
139136
MockOrganization,
140137
{
141138
editOrganization: false,
142-
viewMembers: false,
143-
viewGroups: false,
144-
auditOrganization: true,
139+
editMembers: false,
140+
editGroups: false,
145141
},
146142
],
147143
],
@@ -160,9 +156,8 @@ export const SelectedOrgUserAdmin: Story = {
160156
MockOrganization,
161157
{
162158
editOrganization: false,
163-
viewMembers: true,
164-
viewGroups: true,
165-
auditOrganization: false,
159+
editMembers: true,
160+
editGroups: true,
166161
},
167162
],
168163
],
@@ -176,18 +171,16 @@ export const MultiOrgAdminAndUserAdmin: Story = {
176171
MockOrganization,
177172
{
178173
editOrganization: false,
179-
viewMembers: false,
180-
viewGroups: false,
181-
auditOrganization: true,
174+
editMembers: false,
175+
editGroups: false,
182176
},
183177
],
184178
[
185179
MockOrganization2,
186180
{
187181
editOrganization: false,
188-
viewMembers: true,
189-
viewGroups: true,
190-
auditOrganization: false,
182+
editMembers: true,
183+
editGroups: true,
191184
},
192185
],
193186
],
@@ -202,18 +195,16 @@ export const SelectedMultiOrgAdminAndUserAdmin: Story = {
202195
MockOrganization,
203196
{
204197
editOrganization: false,
205-
viewMembers: false,
206-
viewGroups: false,
207-
auditOrganization: true,
198+
editMembers: false,
199+
editGroups: false,
208200
},
209201
],
210202
[
211203
MockOrganization2,
212204
{
213205
editOrganization: false,
214-
viewMembers: true,
215-
viewGroups: true,
216-
auditOrganization: false,
206+
editMembers: true,
207+
editGroups: true,
217208
},
218209
],
219210
],

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

Lines changed: 3 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import { Sidebar as BaseSidebar } from "components/Sidebar/Sidebar";
1010
import { Stack } from "components/Stack/Stack";
1111
import { UserAvatar } from "components/UserAvatar/UserAvatar";
1212
import { type ClassName, useClassName } from "hooks/useClassName";
13-
import { linkToAuditing, linkToUsers, withFilter } from "modules/navigation";
13+
import { linkToAuditing, linkToUsers } from "modules/navigation";
1414

1515
interface SidebarProps {
1616
/** True if a settings page is being viewed. */
@@ -234,33 +234,20 @@ 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
>
248248
Groups
249249
</SidebarNavSubItem>
250250
)}
251-
{/* For now redirect to the site-wide audit page with the organization
252-
pre-filled into the filter. Based on user feedback we might want
253-
to serve a copy of the audit page or even delete this link. */}
254-
{props.permissions.auditOrganization && (
255-
<SidebarNavSubItem
256-
href={`/deployment${withFilter(
257-
linkToAuditing,
258-
`organization:${props.organization.name}`,
259-
)}`}
260-
>
261-
Auditing
262-
</SidebarNavSubItem>
263-
)}
264251
</Stack>
265252
)}
266253
</>

0 commit comments

Comments
 (0)
0