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.
coder · code-asher · Aug 9, 2024 · Aug 6, 2024 · Aug 6, 2024 · Aug 7, 2024
commit 475f6e238abfaef86214db8aae130f6c79dfdc3e
diff --git a/site/src/api/queries/organizations.ts b/site/src/api/queries/organizations.ts
@@ -1,7 +1,6 @@
 import type { QueryClient } from "react-query";
 import { API } from "api/api";
 import type {
-  AuthorizationCheck,
   AuthorizationResponse,
   CreateOrganizationRequest,
   Organization,
@@ -124,60 +123,6 @@ export const provisionerDaemons = (organization: string) => {
   };
 };
 
-const orgChecks = (
-  organizationId: string,
-): Record<string, AuthorizationCheck> => ({
-  viewMembers: {
-    object: {
-      resource_type: "organization_member",
-      organization_id: organizationId,
-    },
-    action: "read",
-  },
-  editMembers: {
-    object: {
-      resource_type: "organization_member",
-      organization_id: organizationId,
-    },
-    action: "update",
-  },
-  createGroup: {
-    object: {
-      resource_type: "group",
-      organization_id: organizationId,
-    },
-    action: "create",
-  },
-  viewGroups: {
-    object: {
-      resource_type: "group",
-      organization_id: organizationId,
-    },
-    action: "read",
-  },
-  editGroups: {
-    object: {
-      resource_type: "group",
-      organization_id: organizationId,
-    },
-    action: "update",
-  },
-  editOrganization: {
-    object: {
-      resource_type: "organization",
-      organization_id: organizationId,
-    },
-    action: "update",
-  },
-  auditOrganization: {
-    object: {
-      resource_type: "audit_log",
-      organization_id: organizationId,
-    },
-    action: "read",
-  },
-});
-
 /**
  * Fetch permissions for a single organization.
  *
@@ -190,7 +135,31 @@ export const organizationPermissions = (organizationId: string | undefined) => {
   return {
     queryKey: ["organization", organizationId, "permissions"],
     queryFn: () =>
-      API.checkAuthorization({ checks: orgChecks(organizationId) }),
+      // Only request what we use on individual org settings, members, and group
+      // pages, which at the moment is whether you can edit the members on the
+      // members page and whether you can see the create group button on the
+      // groups page.  The edit organization check for the settings page is
+      // covered by the multi-org query at the moment, and the edit group check
+      // on the group page is done on the group itself, not the org, so neither
+      // show up here.
+      API.checkAuthorization({
+        checks: {
+          editMembers: {
+            object: {
+              resource_type: "organization_member",
+              organization_id: organizationId,
+            },
+            action: "update",
+          },
+          createGroup: {
+            object: {
+              resource_type: "group",
+              organization_id: organizationId,
+            },
+            action: "create",
+          },
+        },
+      }),
   };
 };
 
@@ -209,19 +178,54 @@ export const organizationsPermissions = (
   return {
     queryKey: ["organizations", "permissions"],
     queryFn: async () => {
+      // Only request what we need for the sidebar, which is one edit permission
+      // per sub-link (audit page, settings page, groups page, and members page)
+      // that tells us whether to show that page, since we only show them if you
+      // can edit (and not, at the moment if you can only view).
+      const checks = (organizationId: string) => ({
+        editMembers: {
+          object: {
+            resource_type: "organization_member",
+            organization_id: organizationId,
+          },
+          action: "update",
+        },
+        editGroups: {
+          object: {
+            resource_type: "group",
+            organization_id: organizationId,
+          },
+          action: "update",
+        },
+        editOrganization: {
+          object: {
+            resource_type: "organization",
+            organization_id: organizationId,
+          },
+          action: "update",
+        },
+        auditOrganization: {
+          object: {
+            resource_type: "audit_log",
+            organization_id: organizationId,
+          },
+          action: "read",
+        },
+      });
+
       // The endpoint takes a flat array, so to avoid collisions prepend each
       // check with the org ID (the key can be anything we want).
-      const checks = organizations
+      const prefixedChecks = organizations
         .map((org) =>
-          Object.entries(orgChecks(org.id)).map(([key, val]) => [
+          Object.entries(checks(org.id)).map(([key, val]) => [
             `${org.id}.${key}`,
             val,
           ]),
         )
         .flat();
 
       const response = await API.checkAuthorization({
-        checks: Object.fromEntries(checks),
+        checks: Object.fromEntries(prefixedChecks),
       });
 
       // Now we can unflatten by parsing out the org ID from each check.

diff --git a/site/src/pages/ManagementSettingsPage/OrganizationMembersPage.test.tsx b/site/src/pages/ManagementSettingsPage/OrganizationMembersPage.test.tsx
@@ -28,7 +28,6 @@ beforeEach(() => {
     http.post("/api/v2/authcheck", async () => {
       return HttpResponse.json({
         editMembers: true,
-        viewMembers: true,
         viewDeploymentValues: true,
       });
     }),

diff --git a/site/src/pages/ManagementSettingsPage/SidebarView.stories.tsx b/site/src/pages/ManagementSettingsPage/SidebarView.stories.tsx
@@ -17,17 +17,17 @@ const meta: Meta<typeof SidebarView> = {
         MockOrganization,
         {
           editOrganization: true,
-          viewMembers: true,
-          viewGroups: true,
+          editMembers: true,
+          editGroups: true,
           auditOrganization: true,
         },
       ],
       [
         MockOrganization2,
         {
           editOrganization: true,
-          viewMembers: true,
-          viewGroups: true,
+          editMembers: true,
+          editGroups: true,
           auditOrganization: true,
         },
       ],
@@ -118,8 +118,8 @@ export const SelectedOrgAdmin: Story = {
         MockOrganization,
         {
           editOrganization: true,
-          viewMembers: true,
-          viewGroups: true,
+          editMembers: true,
+          editGroups: true,
           auditOrganization: true,
         },
       ],
@@ -139,8 +139,8 @@ export const SelectedOrgAuditor: Story = {
         MockOrganization,
         {
           editOrganization: false,
-          viewMembers: false,
-          viewGroups: false,
+          editMembers: false,
+          editGroups: false,
           auditOrganization: true,
         },
       ],
@@ -160,8 +160,8 @@ export const SelectedOrgUserAdmin: Story = {
         MockOrganization,
         {
           editOrganization: false,
-          viewMembers: true,
-          viewGroups: true,
+          editMembers: true,
+          editGroups: true,
           auditOrganization: false,
         },
       ],
@@ -176,17 +176,17 @@ export const MultiOrgAdminAndUserAdmin: Story = {
         MockOrganization,
         {
           editOrganization: false,
-          viewMembers: false,
-          viewGroups: false,
+          editMembers: false,
+          editGroups: false,
           auditOrganization: true,
         },
       ],
       [
         MockOrganization2,
         {
           editOrganization: false,
-          viewMembers: true,
-          viewGroups: true,
+          editMembers: true,
+          editGroups: true,
           auditOrganization: false,
         },
       ],
@@ -202,17 +202,17 @@ export const SelectedMultiOrgAdminAndUserAdmin: Story = {
         MockOrganization,
         {
           editOrganization: false,
-          viewMembers: false,
-          viewGroups: false,
+          editMembers: false,
+          editGroups: false,
           auditOrganization: true,
         },
       ],
       [
         MockOrganization2,
         {
           editOrganization: false,
-          viewMembers: true,
-          viewGroups: true,
+          editMembers: true,
+          editGroups: true,
           auditOrganization: false,
         },
       ],

diff --git a/site/src/pages/ManagementSettingsPage/SidebarView.tsx b/site/src/pages/ManagementSettingsPage/SidebarView.tsx
@@ -234,14 +234,14 @@ const OrganizationSettingsNavigation: FC<
               Organization settings
             </SidebarNavSubItem>
           )}
-          {props.permissions.viewMembers && (
+          {props.permissions.editMembers && (
             <SidebarNavSubItem
               href={urlForSubpage(props.organization.name, "members")}
             >
               Members
             </SidebarNavSubItem>
           )}
-          {props.permissions.viewGroups && (
+          {props.permissions.editGroups && (
             <SidebarNavSubItem
               href={urlForSubpage(props.organization.name, "groups")}
             >