From 713b9546efd2ad59ba595cf47d1c9ab76dfd7569 Mon Sep 17 00:00:00 2001 From: Asher Date: Tue, 6 Aug 2024 13:23:03 -0800 Subject: [PATCH 01/13] fix: only show editable orgs on deployment page Also make sure the redirect from /organizations goes to an org that the user can edit, rather than always the default org. --- site/src/api/queries/organizations.ts | 144 +++++++++++----- .../ManagementSettingsLayout.tsx | 16 +- .../OrganizationSettingsPage.test.tsx | 125 ++++++++++++++ .../OrganizationSettingsPage.tsx | 44 ++--- .../OrganizationSettingsPageView.tsx | 1 + .../pages/ManagementSettingsPage/Sidebar.tsx | 42 +++-- .../SidebarView.stories.tsx | 156 +++++++++++++++--- .../ManagementSettingsPage/SidebarView.tsx | 119 +++++++------ 8 files changed, 493 insertions(+), 154 deletions(-) create mode 100644 site/src/pages/ManagementSettingsPage/OrganizationSettingsPage.test.tsx diff --git a/site/src/api/queries/organizations.ts b/site/src/api/queries/organizations.ts index 65cb843c08e2e..d0ab2a8bfad7f 100644 --- a/site/src/api/queries/organizations.ts +++ b/site/src/api/queries/organizations.ts @@ -1,7 +1,10 @@ import type { QueryClient } from "react-query"; import { API } from "api/api"; import type { + AuthorizationCheck, + AuthorizationResponse, CreateOrganizationRequest, + Organization, UpdateOrganizationRequest, } from "api/typesGenerated"; import { meKey } from "./users"; @@ -121,6 +124,53 @@ export const provisionerDaemons = (organization: string) => { }; }; +const orgChecks = ( + organizationId: string, +): Record => ({ + 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", + }, + 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. * @@ -133,51 +183,55 @@ export const organizationPermissions = (organizationId: string | undefined) => { return { queryKey: ["organization", organizationId, "permissions"], queryFn: () => - API.checkAuthorization({ - checks: { - 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", - }, - editOrganization: { - object: { - resource_type: "organization", - organization_id: organizationId, - }, - action: "update", - }, - auditOrganization: { - object: { - resource_type: "audit_log", - organization_id: organizationId, - }, - action: "read", - }, + API.checkAuthorization({ checks: orgChecks(organizationId) }), + }; +}; + +/** + * Fetch permissions for all provided organizations. + * + * If organizations are undefined, return a disabled query. + */ +export const organizationsPermissions = ( + organizations: Organization[] | undefined, +) => { + if (!organizations) { + return { enabled: false }; + } + + return { + queryKey: ["organizations", "permissions"], + queryFn: async () => { + // 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 + .map((org) => + Object.entries(orgChecks(org.id)).map(([key, val]) => [ + `${org.id}.${key}`, + val, + ]), + ) + .flat(); + + const response = await API.checkAuthorization({ + checks: Object.fromEntries(checks), + }); + + // Now we can unflatten by parsing out the org ID from each check. + return Object.entries(response).reduce( + (acc, [key, value]) => { + const index = key.indexOf("."); + const orgId = key.substring(0, index); + const perm = key.substring(index + 1); + if (!acc[orgId]) { + acc[orgId] = { [perm]: value }; + } else { + acc[orgId][perm] = value; + } + return acc; }, - }), + {} as Record, + ); + }, }; }; diff --git a/site/src/pages/ManagementSettingsPage/ManagementSettingsLayout.tsx b/site/src/pages/ManagementSettingsPage/ManagementSettingsLayout.tsx index eb727919984ba..f019a92218904 100644 --- a/site/src/pages/ManagementSettingsPage/ManagementSettingsLayout.tsx +++ b/site/src/pages/ManagementSettingsPage/ManagementSettingsLayout.tsx @@ -2,7 +2,7 @@ import { type FC, Suspense } from "react"; import { useQuery } from "react-query"; import { Outlet } from "react-router-dom"; import { deploymentConfig } from "api/queries/deployment"; -import type { Organization } from "api/typesGenerated"; +import type { AuthorizationResponse, Organization } from "api/typesGenerated"; import { Loader } from "components/Loader/Loader"; import { Margins } from "components/Margins/Margins"; import { Stack } from "components/Stack/Stack"; @@ -21,6 +21,20 @@ export const useOrganizationSettings = (): OrganizationSettingsValue => { return { organizations }; }; +/** + * Return true if the user can edit the organization settings or its members. + */ +export const canEditOrganization = ( + permissions: AuthorizationResponse | undefined, +) => { + return ( + permissions && + (permissions.editOrganization || + permissions.viewMembers || + permissions.viewGroups) + ); +}; + /** * A multi-org capable settings page layout. * diff --git a/site/src/pages/ManagementSettingsPage/OrganizationSettingsPage.test.tsx b/site/src/pages/ManagementSettingsPage/OrganizationSettingsPage.test.tsx new file mode 100644 index 0000000000000..a32682a33aac3 --- /dev/null +++ b/site/src/pages/ManagementSettingsPage/OrganizationSettingsPage.test.tsx @@ -0,0 +1,125 @@ +import { screen, within } from "@testing-library/react"; +import { HttpResponse, http } from "msw"; +import { + MockDefaultOrganization, + MockOrganization2, +} from "testHelpers/entities"; +import { + renderWithManagementSettingsLayout, + waitForLoaderToBeRemoved, +} from "testHelpers/renderHelpers"; +import { server } from "testHelpers/server"; +import OrganizationSettingsPage from "./OrganizationSettingsPage"; + +jest.spyOn(console, "error").mockImplementation(() => {}); + +const renderRootPage = async () => { + renderWithManagementSettingsLayout(, { + route: "/organizations", + path: "/organizations", + extraRoutes: [ + { + path: "/organizations/:organization", + element: , + }, + ], + }); + await waitForLoaderToBeRemoved(); +}; + +const renderPage = async (orgName: string) => { + renderWithManagementSettingsLayout(, { + route: `/organizations/${orgName}`, + path: "/organizations/:organization", + }); + await waitForLoaderToBeRemoved(); +}; + +describe("OrganizationSettingsPage", () => { + it("has no organizations", async () => { + server.use( + http.get("/api/v2/organizations", () => { + return HttpResponse.json([]); + }), + http.post("/api/v2/authcheck", async () => { + return HttpResponse.json({ + [`${MockDefaultOrganization.id}.editOrganization`]: true, + viewDeploymentValues: true, + }); + }), + ); + await renderRootPage(); + await screen.findByText("No organizations found"); + }); + + it("has no editable organizations", async () => { + server.use( + http.get("/api/v2/organizations", () => { + return HttpResponse.json([MockDefaultOrganization, MockOrganization2]); + }), + http.post("/api/v2/authcheck", async () => { + return HttpResponse.json({ + viewDeploymentValues: true, + }); + }), + ); + await renderRootPage(); + await screen.findByText("No organizations found"); + }); + + it("redirects to default organization", async () => { + server.use( + http.get("/api/v2/organizations", () => { + // Default always preferred regardless of order. + return HttpResponse.json([MockOrganization2, MockDefaultOrganization]); + }), + http.post("/api/v2/authcheck", async () => { + return HttpResponse.json({ + [`${MockDefaultOrganization.id}.editOrganization`]: true, + [`${MockOrganization2.id}.editOrganization`]: true, + viewDeploymentValues: true, + }); + }), + ); + await renderRootPage(); + const form = screen.getByTestId("org-settings-form"); + expect(within(form).getByRole("textbox", { name: "Name" })).toHaveValue( + MockDefaultOrganization.name, + ); + }); + + it("redirects to non-default organization", async () => { + server.use( + http.get("/api/v2/organizations", () => { + return HttpResponse.json([MockDefaultOrganization, MockOrganization2]); + }), + http.post("/api/v2/authcheck", async () => { + return HttpResponse.json({ + [`${MockOrganization2.id}.editOrganization`]: true, + viewDeploymentValues: true, + }); + }), + ); + await renderRootPage(); + const form = screen.getByTestId("org-settings-form"); + expect(within(form).getByRole("textbox", { name: "Name" })).toHaveValue( + MockOrganization2.name, + ); + }); + + it("cannot find organization", async () => { + server.use( + http.get("/api/v2/organizations", () => { + return HttpResponse.json([MockDefaultOrganization, MockOrganization2]); + }), + http.post("/api/v2/authcheck", async () => { + return HttpResponse.json({ + [`${MockOrganization2.id}.editOrganization`]: true, + viewDeploymentValues: true, + }); + }), + ); + await renderPage("the-endless-void"); + await screen.findByText("Organization not found"); + }); +}); diff --git a/site/src/pages/ManagementSettingsPage/OrganizationSettingsPage.tsx b/site/src/pages/ManagementSettingsPage/OrganizationSettingsPage.tsx index 74fb294e2629a..bc62a5423c73c 100644 --- a/site/src/pages/ManagementSettingsPage/OrganizationSettingsPage.tsx +++ b/site/src/pages/ManagementSettingsPage/OrganizationSettingsPage.tsx @@ -4,13 +4,16 @@ import { Navigate, useNavigate, useParams } from "react-router-dom"; import { updateOrganization, deleteOrganization, - organizationPermissions, + organizationsPermissions, } from "api/queries/organizations"; import type { Organization } from "api/typesGenerated"; import { EmptyState } from "components/EmptyState/EmptyState"; import { displaySuccess } from "components/GlobalSnackbar/utils"; import { Loader } from "components/Loader/Loader"; -import { useOrganizationSettings } from "./ManagementSettingsLayout"; +import { + canEditOrganization, + useOrganizationSettings, +} from "./ManagementSettingsLayout"; import { OrganizationSettingsPageView } from "./OrganizationSettingsPageView"; const OrganizationSettingsPage: FC = () => { @@ -32,37 +35,43 @@ const OrganizationSettingsPage: FC = () => { organizations && organizationName ? getOrganizationByName(organizations, organizationName) : undefined; - const permissionsQuery = useQuery(organizationPermissions(organization?.id)); + const permissionsQuery = useQuery(organizationsPermissions(organizations)); - if (!organizations) { + const permissions = permissionsQuery.data; + if (!organizations || !permissions) { return ; } - // Redirect /organizations => /organizations/default-org + // Redirect /organizations => /organizations/default-org, or if they cannot edit + // the default org, then the first org they can edit, if any. if (!organizationName) { - const defaultOrg = getOrganizationByDefault(organizations); - if (defaultOrg) { - return ; + const editableOrg = organizations + .sort((a, b) => { + // Prefer default org (it may not be first). + if (a.is_default) { + return -1; + } else if (b.is_default) { + return 1; + } + return 0; + }) + .find((org) => canEditOrganization(permissions[org.id])); + if (editableOrg) { + return ; } - // We expect there to always be a default organization. - throw new Error("No default organization found"); + return ; } if (!organization) { return ; } - const permissions = permissionsQuery.data; - if (!permissions) { - return ; - } - const error = updateOrganizationMutation.error ?? deleteOrganizationMutation.error; return ( { @@ -85,8 +94,5 @@ const OrganizationSettingsPage: FC = () => { export default OrganizationSettingsPage; -const getOrganizationByDefault = (organizations: Organization[]) => - organizations.find((org) => org.is_default); - const getOrganizationByName = (organizations: Organization[], name: string) => organizations.find((org) => org.name === name); diff --git a/site/src/pages/ManagementSettingsPage/OrganizationSettingsPageView.tsx b/site/src/pages/ManagementSettingsPage/OrganizationSettingsPageView.tsx index 9f8192117bfb6..be2a5e7cf2365 100644 --- a/site/src/pages/ManagementSettingsPage/OrganizationSettingsPageView.tsx +++ b/site/src/pages/ManagementSettingsPage/OrganizationSettingsPageView.tsx @@ -78,6 +78,7 @@ export const OrganizationSettingsPageView: FC< )} diff --git a/site/src/pages/ManagementSettingsPage/Sidebar.tsx b/site/src/pages/ManagementSettingsPage/Sidebar.tsx index 8e2f80652d93b..3d91d3b99955d 100644 --- a/site/src/pages/ManagementSettingsPage/Sidebar.tsx +++ b/site/src/pages/ManagementSettingsPage/Sidebar.tsx @@ -1,9 +1,13 @@ import type { FC } from "react"; import { useQuery } from "react-query"; -import { useParams } from "react-router-dom"; -import { organizationPermissions } from "api/queries/organizations"; +import { useLocation, useParams } from "react-router-dom"; +import { organizationsPermissions } from "api/queries/organizations"; +import type { AuthorizationResponse, Organization } from "api/typesGenerated"; import { useAuthenticated } from "contexts/auth/RequireAuth"; -import { useOrganizationSettings } from "./ManagementSettingsLayout"; +import { + canEditOrganization, + useOrganizationSettings, +} from "./ManagementSettingsLayout"; import { SidebarView } from "./SidebarView"; /** @@ -14,27 +18,35 @@ import { SidebarView } from "./SidebarView"; * DeploySettingsPage/Sidebar instead. */ export const Sidebar: FC = () => { + const location = useLocation(); const { permissions } = useAuthenticated(); const { organizations } = useOrganizationSettings(); const { organization: organizationName } = useParams() as { organization?: string; }; - // If there is no organization name, the settings page will load, and it will - // redirect to the default organization, so eventually there will always be an - // organization name. - const activeOrganization = organizations?.find( - (o) => o.name === organizationName, - ); - const activeOrgPermissionsQuery = useQuery( - organizationPermissions(activeOrganization?.id), - ); + const orgPermissionsQuery = useQuery(organizationsPermissions(organizations)); + + // Sometimes a user can read an organization but cannot actually do anything + // with it. For now, these are filtered out so you only see organizations you + // can manage in some way. + const editableOrgs = organizations + ?.map((org) => { + const permissions = orgPermissionsQuery.data?.[org.id]; + return [org, permissions] as [Organization, AuthorizationResponse]; + }) + .filter(([_, permissions]) => { + return canEditOrganization(permissions); + }); return ( ); diff --git a/site/src/pages/ManagementSettingsPage/SidebarView.stories.tsx b/site/src/pages/ManagementSettingsPage/SidebarView.stories.tsx index 5490b4df68b49..22d73c694d78a 100644 --- a/site/src/pages/ManagementSettingsPage/SidebarView.stories.tsx +++ b/site/src/pages/ManagementSettingsPage/SidebarView.stories.tsx @@ -10,9 +10,28 @@ const meta: Meta = { title: "components/MultiOrgSidebarView", component: SidebarView, args: { - activeOrganization: undefined, - activeOrgPermissions: undefined, - organizations: [MockOrganization, MockOrganization2], + activeSettings: true, + activeOrganizationName: undefined, + organizations: [ + [ + MockOrganization, + { + editOrganization: true, + viewMembers: true, + viewGroups: true, + auditOrganization: true, + }, + ], + [ + MockOrganization2, + { + editOrganization: true, + viewMembers: true, + viewGroups: true, + auditOrganization: true, + }, + ], + ], permissions: MockPermissions, }, }; @@ -20,7 +39,11 @@ const meta: Meta = { export default meta; type Story = StoryObj; -export const Default: Story = {}; +export const LoadingOrganizations: Story = { + args: { + organizations: undefined, + }, +}; export const NoCreateOrg: Story = { args: { @@ -74,44 +97,125 @@ export const NoPermissions: Story = { }, }; -export const SelectedOrgLoading: Story = { +export const NoSelected: Story = { args: { - activeOrganization: MockOrganization, + activeSettings: false, + }, +}; + +export const SelectedOrgNoMatch: Story = { + args: { + activeOrganizationName: MockOrganization.name, + organizations: [], }, }; export const SelectedOrgAdmin: Story = { args: { - activeOrganization: MockOrganization, - activeOrgPermissions: { - editOrganization: true, - viewMembers: true, - viewGroups: true, - auditOrganization: true, - }, + activeOrganizationName: MockOrganization.name, + organizations: [ + [ + MockOrganization, + { + editOrganization: true, + viewMembers: true, + viewGroups: true, + auditOrganization: true, + }, + ], + ], }, }; export const SelectedOrgAuditor: Story = { args: { - activeOrganization: MockOrganization, - activeOrgPermissions: { - editOrganization: false, - viewMembers: false, - viewGroups: false, - auditOrganization: true, + activeOrganizationName: MockOrganization.name, + permissions: { + ...MockPermissions, + createOrganization: false, }, + organizations: [ + [ + MockOrganization, + { + editOrganization: false, + viewMembers: false, + viewGroups: false, + auditOrganization: true, + }, + ], + ], }, }; -export const SelectedOrgNoPerms: Story = { +export const SelectedOrgUserAdmin: Story = { args: { - activeOrganization: MockOrganization, - activeOrgPermissions: { - editOrganization: false, - viewMembers: false, - viewGroups: false, - auditOrganization: false, + activeOrganizationName: MockOrganization.name, + permissions: { + ...MockPermissions, + createOrganization: false, }, + organizations: [ + [ + MockOrganization, + { + editOrganization: false, + viewMembers: true, + viewGroups: true, + auditOrganization: false, + }, + ], + ], + }, +}; + +export const MultiOrgAdminAndUserAdmin: Story = { + args: { + organizations: [ + [ + MockOrganization, + { + editOrganization: false, + viewMembers: false, + viewGroups: false, + auditOrganization: true, + }, + ], + [ + MockOrganization2, + { + editOrganization: false, + viewMembers: true, + viewGroups: true, + auditOrganization: false, + }, + ], + ], + }, +}; + +export const SelectedMultiOrgAdminAndUserAdmin: Story = { + args: { + activeOrganizationName: MockOrganization2.name, + organizations: [ + [ + MockOrganization, + { + editOrganization: false, + viewMembers: false, + viewGroups: false, + auditOrganization: true, + }, + ], + [ + MockOrganization2, + { + editOrganization: false, + viewMembers: true, + viewGroups: true, + auditOrganization: false, + }, + ], + ], }, }; diff --git a/site/src/pages/ManagementSettingsPage/SidebarView.tsx b/site/src/pages/ManagementSettingsPage/SidebarView.tsx index d9738563aa9de..7109924bd5312 100644 --- a/site/src/pages/ManagementSettingsPage/SidebarView.tsx +++ b/site/src/pages/ManagementSettingsPage/SidebarView.tsx @@ -13,18 +13,12 @@ import { type ClassName, useClassName } from "hooks/useClassName"; import { linkToAuditing, linkToUsers, withFilter } from "modules/navigation"; interface SidebarProps { - /** - * The active org if an org is being viewed. If there is no active - * organization, assume one of the deployment settings pages are being viewed. - */ - activeOrganization: Organization | undefined; - /** - * The permissions for the active org or undefined if still fetching (or if - * there is no active org). - */ - activeOrgPermissions: AuthorizationResponse | undefined; - /** The list of organizations or undefined if still fetching. */ - organizations: Organization[] | undefined; + /** True if a settings page is being viewed. */ + activeSettings: boolean; + /** The active org name, if any. Overrides activeSettings. */ + activeOrganizationName: string | undefined; + /** Organizations and their permissions or undefined if still fetching. */ + organizations: [Organization, AuthorizationResponse][] | undefined; /** Site-wide permissions. */ permissions: AuthorizationResponse; } @@ -38,38 +32,10 @@ export const SidebarView: FC = (props) => {
Deployment
- {props.organizations ? ( - <> -
Organizations
- {props.permissions.createOrganization && ( - } - > - New organization - - )} - {props.organizations.map((org) => { - const orgActive = - Boolean(props.activeOrganization) && - org.name === props.activeOrganization?.name; - return ( - - ); - })} - - ) : ( - - )} + ); }; @@ -167,19 +133,77 @@ function urlForSubpage(organizationName: string, subpage: string = ""): string { return `/organizations/${organizationName}/${subpage}`; } +interface OrganizationsSettingsNavigationProps { + /** The active org name if an org is being viewed. */ + activeOrganizationName: string | undefined; + /** Organizations and their permissions or undefined if still fetching. */ + organizations: [Organization, AuthorizationResponse][] | undefined; + /** Site-wide permissions. */ + permissions: AuthorizationResponse; +} + +/** + * Displays navigation for all organizations and a create organization link. + * + * If organizations or their permissions are still loading, show a loader. + * + * If there are no organizations and the user does not have the create org + * permission, nothing is displayed. + */ +const OrganizationsSettingsNavigation: FC< + OrganizationsSettingsNavigationProps +> = (props) => { + // Wait for organizations and their permissions to load in. + if (!props.organizations) { + return ; + } + + if ( + props.organizations.length <= 0 && + !props.permissions.createOrganization + ) { + return null; + } + + return ( + <> +
Organizations
+ {props.permissions.createOrganization && ( + } + > + New organization + + )} + {props.organizations.map(([org, permissions]) => ( + + ))} + + ); +}; + interface OrganizationSettingsNavigationProps { + /** Whether this organization is currently selected. */ active: boolean; + /** The organization to display in the navigation. */ organization: Organization; - permissions: AuthorizationResponse | undefined; + /** The permissions for this organization. */ + permissions: AuthorizationResponse; } /** - * Displays navigation for an organization. + * Displays navigation for a single organization. * * If inactive, no sub-menu items will be shown, just the organization name. * - * If active, it will show a loader until the permissions are defined, then the - * sub-menu items are shown as appropriate. + * If active, it will show sub-menu items based on the permissions. */ const OrganizationSettingsNavigation: FC< OrganizationSettingsNavigationProps @@ -200,8 +224,7 @@ const OrganizationSettingsNavigation: FC< > {props.organization.display_name} - {props.active && !props.permissions && } - {props.active && props.permissions && ( + {props.active && ( {props.permissions.editOrganization && ( Date: Tue, 6 Aug 2024 15:52:16 -0800 Subject: [PATCH 02/13] canEditOrganization was lying --- site/src/api/queries/organizations.ts | 7 +++++++ .../ManagementSettingsPage/ManagementSettingsLayout.tsx | 4 ++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/site/src/api/queries/organizations.ts b/site/src/api/queries/organizations.ts index d0ab2a8bfad7f..b0d0d89b77179 100644 --- a/site/src/api/queries/organizations.ts +++ b/site/src/api/queries/organizations.ts @@ -155,6 +155,13 @@ const orgChecks = ( }, action: "read", }, + editGroups: { + object: { + resource_type: "group", + organization_id: organizationId, + }, + action: "update", + }, editOrganization: { object: { resource_type: "organization", diff --git a/site/src/pages/ManagementSettingsPage/ManagementSettingsLayout.tsx b/site/src/pages/ManagementSettingsPage/ManagementSettingsLayout.tsx index f019a92218904..8fb08330e5f73 100644 --- a/site/src/pages/ManagementSettingsPage/ManagementSettingsLayout.tsx +++ b/site/src/pages/ManagementSettingsPage/ManagementSettingsLayout.tsx @@ -30,8 +30,8 @@ export const canEditOrganization = ( return ( permissions && (permissions.editOrganization || - permissions.viewMembers || - permissions.viewGroups) + permissions.editMembers || + permissions.editGroups) ); }; From 475f6e238abfaef86214db8aae130f6c79dfdc3e Mon Sep 17 00:00:00 2001 From: Asher Date: Wed, 7 Aug 2024 12:05:28 -0800 Subject: [PATCH 03/13] 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. --- site/src/api/queries/organizations.ts | 122 +++++++++--------- .../OrganizationMembersPage.test.tsx | 1 - .../SidebarView.stories.tsx | 36 +++--- .../ManagementSettingsPage/SidebarView.tsx | 4 +- 4 files changed, 83 insertions(+), 80 deletions(-) diff --git a/site/src/api/queries/organizations.ts b/site/src/api/queries/organizations.ts index b0d0d89b77179..a9f596c900ef3 100644 --- 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 => ({ - 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,11 +178,46 @@ 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, ]), @@ -221,7 +225,7 @@ export const organizationsPermissions = ( .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 index 2bf459d0d0509..9b78cf4e65121 100644 --- 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 index 22d73c694d78a..9df63608debc4 100644 --- a/site/src/pages/ManagementSettingsPage/SidebarView.stories.tsx +++ b/site/src/pages/ManagementSettingsPage/SidebarView.stories.tsx @@ -17,8 +17,8 @@ const meta: Meta = { MockOrganization, { editOrganization: true, - viewMembers: true, - viewGroups: true, + editMembers: true, + editGroups: true, auditOrganization: true, }, ], @@ -26,8 +26,8 @@ const meta: Meta = { 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,8 +176,8 @@ export const MultiOrgAdminAndUserAdmin: Story = { MockOrganization, { editOrganization: false, - viewMembers: false, - viewGroups: false, + editMembers: false, + editGroups: false, auditOrganization: true, }, ], @@ -185,8 +185,8 @@ export const MultiOrgAdminAndUserAdmin: Story = { MockOrganization2, { editOrganization: false, - viewMembers: true, - viewGroups: true, + editMembers: true, + editGroups: true, auditOrganization: false, }, ], @@ -202,8 +202,8 @@ export const SelectedMultiOrgAdminAndUserAdmin: Story = { MockOrganization, { editOrganization: false, - viewMembers: false, - viewGroups: false, + editMembers: false, + editGroups: false, auditOrganization: true, }, ], @@ -211,8 +211,8 @@ export const SelectedMultiOrgAdminAndUserAdmin: Story = { 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 index 7109924bd5312..f07c79d458de0 100644 --- a/site/src/pages/ManagementSettingsPage/SidebarView.tsx +++ b/site/src/pages/ManagementSettingsPage/SidebarView.tsx @@ -234,14 +234,14 @@ const OrganizationSettingsNavigation: FC< Organization settings )} - {props.permissions.viewMembers && ( + {props.permissions.editMembers && ( Members )} - {props.permissions.viewGroups && ( + {props.permissions.editGroups && ( From 83bf388ddd25919dd0831843ed333e54cc5087a3 Mon Sep 17 00:00:00 2001 From: Asher Date: Wed, 7 Aug 2024 14:38:29 -0800 Subject: [PATCH 04/13] fixup! fix: only show editable orgs on deployment page --- .../pages/ManagementSettingsPage/OrganizationSettingsPage.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/site/src/pages/ManagementSettingsPage/OrganizationSettingsPage.tsx b/site/src/pages/ManagementSettingsPage/OrganizationSettingsPage.tsx index bc62a5423c73c..e676460954dc0 100644 --- a/site/src/pages/ManagementSettingsPage/OrganizationSettingsPage.tsx +++ b/site/src/pages/ManagementSettingsPage/OrganizationSettingsPage.tsx @@ -71,7 +71,7 @@ const OrganizationSettingsPage: FC = () => { return ( { From e4f5bf7f0c4c03e76bb73567ed2b8e38f357ec58 Mon Sep 17 00:00:00 2001 From: Asher Date: Wed, 7 Aug 2024 16:58:24 -0800 Subject: [PATCH 05/13] Pass in ids instead of entire orgs --- site/src/api/queries/organizations.ts | 13 ++++++------- .../OrganizationSettingsPage.tsx | 4 +++- site/src/pages/ManagementSettingsPage/Sidebar.tsx | 4 +++- 3 files changed, 12 insertions(+), 9 deletions(-) diff --git a/site/src/api/queries/organizations.ts b/site/src/api/queries/organizations.ts index a9f596c900ef3..88f85b38b31a2 100644 --- a/site/src/api/queries/organizations.ts +++ b/site/src/api/queries/organizations.ts @@ -3,7 +3,6 @@ import { API } from "api/api"; import type { AuthorizationResponse, CreateOrganizationRequest, - Organization, UpdateOrganizationRequest, } from "api/typesGenerated"; import { meKey } from "./users"; @@ -169,9 +168,9 @@ export const organizationPermissions = (organizationId: string | undefined) => { * If organizations are undefined, return a disabled query. */ export const organizationsPermissions = ( - organizations: Organization[] | undefined, + organizationIds: string[] | undefined, ) => { - if (!organizations) { + if (!organizationIds) { return { enabled: false }; } @@ -215,10 +214,10 @@ export const organizationsPermissions = ( // 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 prefixedChecks = organizations - .map((org) => - Object.entries(checks(org.id)).map(([key, val]) => [ - `${org.id}.${key}`, + const prefixedChecks = organizationIds + .map((orgId) => + Object.entries(checks(orgId)).map(([key, val]) => [ + `${orgId}.${key}`, val, ]), ) diff --git a/site/src/pages/ManagementSettingsPage/OrganizationSettingsPage.tsx b/site/src/pages/ManagementSettingsPage/OrganizationSettingsPage.tsx index e676460954dc0..a0878eeabd304 100644 --- a/site/src/pages/ManagementSettingsPage/OrganizationSettingsPage.tsx +++ b/site/src/pages/ManagementSettingsPage/OrganizationSettingsPage.tsx @@ -35,7 +35,9 @@ const OrganizationSettingsPage: FC = () => { organizations && organizationName ? getOrganizationByName(organizations, organizationName) : undefined; - const permissionsQuery = useQuery(organizationsPermissions(organizations)); + const permissionsQuery = useQuery( + organizationsPermissions(organizations?.map((o) => o.id)), + ); const permissions = permissionsQuery.data; if (!organizations || !permissions) { diff --git a/site/src/pages/ManagementSettingsPage/Sidebar.tsx b/site/src/pages/ManagementSettingsPage/Sidebar.tsx index 3d91d3b99955d..d09f686705da2 100644 --- a/site/src/pages/ManagementSettingsPage/Sidebar.tsx +++ b/site/src/pages/ManagementSettingsPage/Sidebar.tsx @@ -25,7 +25,9 @@ export const Sidebar: FC = () => { organization?: string; }; - const orgPermissionsQuery = useQuery(organizationsPermissions(organizations)); + const orgPermissionsQuery = useQuery( + organizationsPermissions(organizations?.map((o) => o.id)), + ); // Sometimes a user can read an organization but cannot actually do anything // with it. For now, these are filtered out so you only see organizations you From 9e76ebcbde12992ef11d0012100c87b32884ed91 Mon Sep 17 00:00:00 2001 From: Asher Date: Wed, 7 Aug 2024 17:04:08 -0800 Subject: [PATCH 06/13] Assign permission value in one spot --- site/src/api/queries/organizations.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/site/src/api/queries/organizations.ts b/site/src/api/queries/organizations.ts index 88f85b38b31a2..05ab5d40d6ce0 100644 --- a/site/src/api/queries/organizations.ts +++ b/site/src/api/queries/organizations.ts @@ -234,10 +234,9 @@ export const organizationsPermissions = ( const orgId = key.substring(0, index); const perm = key.substring(index + 1); if (!acc[orgId]) { - acc[orgId] = { [perm]: value }; - } else { - acc[orgId][perm] = value; + acc[orgId] = {}; } + acc[orgId][perm] = value; return acc; }, {} as Record, From bfa0000d2c0a1c9ff28f2dd181e08fd161417137 Mon Sep 17 00:00:00 2001 From: Asher Date: Wed, 7 Aug 2024 17:07:33 -0800 Subject: [PATCH 07/13] Improved sort --- .../ManagementSettingsPage/OrganizationSettingsPage.tsx | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/site/src/pages/ManagementSettingsPage/OrganizationSettingsPage.tsx b/site/src/pages/ManagementSettingsPage/OrganizationSettingsPage.tsx index a0878eeabd304..0b04b3848ed92 100644 --- a/site/src/pages/ManagementSettingsPage/OrganizationSettingsPage.tsx +++ b/site/src/pages/ManagementSettingsPage/OrganizationSettingsPage.tsx @@ -50,12 +50,9 @@ const OrganizationSettingsPage: FC = () => { const editableOrg = organizations .sort((a, b) => { // Prefer default org (it may not be first). - if (a.is_default) { - return -1; - } else if (b.is_default) { - return 1; - } - return 0; + // JavaScript will happily subtract booleans, but use numbers to keep + // the compiler happy. + return (b.is_default ? 1 : 0) - (a.is_default ? 1 : 0); }) .find((org) => canEditOrganization(permissions[org.id])); if (editableOrg) { From b86684d26b199f000c8d3eae168fce5e7cb688cd Mon Sep 17 00:00:00 2001 From: Asher Date: Wed, 7 Aug 2024 17:11:33 -0800 Subject: [PATCH 08/13] Tuples-b-gone --- .../pages/ManagementSettingsPage/Sidebar.tsx | 19 +++-- .../SidebarView.stories.tsx | 72 +++++++++---------- .../ManagementSettingsPage/SidebarView.tsx | 23 +++--- 3 files changed, 61 insertions(+), 53 deletions(-) diff --git a/site/src/pages/ManagementSettingsPage/Sidebar.tsx b/site/src/pages/ManagementSettingsPage/Sidebar.tsx index d09f686705da2..be870d7791d85 100644 --- a/site/src/pages/ManagementSettingsPage/Sidebar.tsx +++ b/site/src/pages/ManagementSettingsPage/Sidebar.tsx @@ -2,13 +2,12 @@ import type { FC } from "react"; import { useQuery } from "react-query"; import { useLocation, useParams } from "react-router-dom"; import { organizationsPermissions } from "api/queries/organizations"; -import type { AuthorizationResponse, Organization } from "api/typesGenerated"; import { useAuthenticated } from "contexts/auth/RequireAuth"; import { canEditOrganization, useOrganizationSettings, } from "./ManagementSettingsLayout"; -import { SidebarView } from "./SidebarView"; +import { type OrganizationWithPermissions, SidebarView } from "./SidebarView"; /** * A combined deployment settings and organization menu. @@ -34,11 +33,19 @@ export const Sidebar: FC = () => { // can manage in some way. const editableOrgs = organizations ?.map((org) => { - const permissions = orgPermissionsQuery.data?.[org.id]; - return [org, permissions] as [Organization, AuthorizationResponse]; + return { + ...org, + permissions: orgPermissionsQuery.data?.[org.id], + }; }) - .filter(([_, permissions]) => { - return canEditOrganization(permissions); + // TypeScript is not able to infer whether permissions are defined from the + // canEditOrganization call, so although redundant this helps figure it out. + .filter( + (org): org is OrganizationWithPermissions => + org.permissions !== undefined, + ) + .filter((org) => { + return canEditOrganization(org.permissions); }); return ( diff --git a/site/src/pages/ManagementSettingsPage/SidebarView.stories.tsx b/site/src/pages/ManagementSettingsPage/SidebarView.stories.tsx index 9df63608debc4..42d21f52d5b70 100644 --- a/site/src/pages/ManagementSettingsPage/SidebarView.stories.tsx +++ b/site/src/pages/ManagementSettingsPage/SidebarView.stories.tsx @@ -13,24 +13,24 @@ const meta: Meta = { activeSettings: true, activeOrganizationName: undefined, organizations: [ - [ - MockOrganization, - { + { + ...MockOrganization, + permissions: { editOrganization: true, editMembers: true, editGroups: true, auditOrganization: true, }, - ], - [ - MockOrganization2, - { + }, + { + ...MockOrganization2, + permissions: { editOrganization: true, editMembers: true, editGroups: true, auditOrganization: true, }, - ], + }, ], permissions: MockPermissions, }, @@ -114,15 +114,15 @@ export const SelectedOrgAdmin: Story = { args: { activeOrganizationName: MockOrganization.name, organizations: [ - [ - MockOrganization, - { + { + ...MockOrganization, + permissions: { editOrganization: true, editMembers: true, editGroups: true, auditOrganization: true, }, - ], + }, ], }, }; @@ -135,15 +135,15 @@ export const SelectedOrgAuditor: Story = { createOrganization: false, }, organizations: [ - [ - MockOrganization, - { + { + ...MockOrganization, + permissions: { editOrganization: false, editMembers: false, editGroups: false, auditOrganization: true, }, - ], + }, ], }, }; @@ -156,15 +156,15 @@ export const SelectedOrgUserAdmin: Story = { createOrganization: false, }, organizations: [ - [ - MockOrganization, - { + { + ...MockOrganization, + permissions: { editOrganization: false, editMembers: true, editGroups: true, auditOrganization: false, }, - ], + }, ], }, }; @@ -172,24 +172,24 @@ export const SelectedOrgUserAdmin: Story = { export const MultiOrgAdminAndUserAdmin: Story = { args: { organizations: [ - [ - MockOrganization, - { + { + ...MockOrganization, + permissions: { editOrganization: false, editMembers: false, editGroups: false, auditOrganization: true, }, - ], - [ - MockOrganization2, - { + }, + { + ...MockOrganization2, + permissions: { editOrganization: false, editMembers: true, editGroups: true, auditOrganization: false, }, - ], + }, ], }, }; @@ -198,24 +198,24 @@ export const SelectedMultiOrgAdminAndUserAdmin: Story = { args: { activeOrganizationName: MockOrganization2.name, organizations: [ - [ - MockOrganization, - { + { + ...MockOrganization, + permissions: { editOrganization: false, editMembers: false, editGroups: false, auditOrganization: true, }, - ], - [ - MockOrganization2, - { + }, + { + ...MockOrganization2, + permissions: { editOrganization: false, editMembers: true, editGroups: true, auditOrganization: false, }, - ], + }, ], }, }; diff --git a/site/src/pages/ManagementSettingsPage/SidebarView.tsx b/site/src/pages/ManagementSettingsPage/SidebarView.tsx index f07c79d458de0..45586df55ae66 100644 --- a/site/src/pages/ManagementSettingsPage/SidebarView.tsx +++ b/site/src/pages/ManagementSettingsPage/SidebarView.tsx @@ -12,13 +12,17 @@ import { UserAvatar } from "components/UserAvatar/UserAvatar"; import { type ClassName, useClassName } from "hooks/useClassName"; import { linkToAuditing, linkToUsers, withFilter } from "modules/navigation"; +export interface OrganizationWithPermissions extends Organization { + permissions: AuthorizationResponse; +} + interface SidebarProps { /** True if a settings page is being viewed. */ activeSettings: boolean; /** The active org name, if any. Overrides activeSettings. */ activeOrganizationName: string | undefined; /** Organizations and their permissions or undefined if still fetching. */ - organizations: [Organization, AuthorizationResponse][] | undefined; + organizations: OrganizationWithPermissions[] | undefined; /** Site-wide permissions. */ permissions: AuthorizationResponse; } @@ -137,7 +141,7 @@ interface OrganizationsSettingsNavigationProps { /** The active org name if an org is being viewed. */ activeOrganizationName: string | undefined; /** Organizations and their permissions or undefined if still fetching. */ - organizations: [Organization, AuthorizationResponse][] | undefined; + organizations: OrganizationWithPermissions[] | undefined; /** Site-wide permissions. */ permissions: AuthorizationResponse; } @@ -177,11 +181,10 @@ const OrganizationsSettingsNavigation: FC< New organization )} - {props.organizations.map(([org, permissions]) => ( + {props.organizations.map((org) => ( ))} @@ -193,9 +196,7 @@ interface OrganizationSettingsNavigationProps { /** Whether this organization is currently selected. */ active: boolean; /** The organization to display in the navigation. */ - organization: Organization; - /** The permissions for this organization. */ - permissions: AuthorizationResponse; + organization: OrganizationWithPermissions; } /** @@ -226,7 +227,7 @@ const OrganizationSettingsNavigation: FC< {props.active && ( - {props.permissions.editOrganization && ( + {props.organization.permissions.editOrganization && ( )} - {props.permissions.editMembers && ( + {props.organization.permissions.editMembers && ( Members )} - {props.permissions.editGroups && ( + {props.organization.permissions.editGroups && ( @@ -251,7 +252,7 @@ const OrganizationSettingsNavigation: FC< {/* For now redirect to the site-wide audit page with the organization pre-filled into the filter. Based on user feedback we might want to serve a copy of the audit page or even delete this link. */} - {props.permissions.auditOrganization && ( + {props.organization.permissions.auditOrganization && ( Date: Wed, 7 Aug 2024 17:25:18 -0800 Subject: [PATCH 09/13] Destruct props --- .../ManagementSettingsPage/SidebarView.tsx | 100 +++++++++--------- 1 file changed, 52 insertions(+), 48 deletions(-) diff --git a/site/src/pages/ManagementSettingsPage/SidebarView.tsx b/site/src/pages/ManagementSettingsPage/SidebarView.tsx index 45586df55ae66..ed3e14e78e756 100644 --- a/site/src/pages/ManagementSettingsPage/SidebarView.tsx +++ b/site/src/pages/ManagementSettingsPage/SidebarView.tsx @@ -30,16 +30,25 @@ interface SidebarProps { /** * A combined deployment settings and organization menu. */ -export const SidebarView: FC = (props) => { +export const SidebarView: FC = ({ + activeSettings, + activeOrganizationName, + organizations, + permissions, +}) => { // TODO: Do something nice to scroll to the active org. return (
Deployment
+ - ); }; @@ -58,15 +67,16 @@ interface DeploymentSettingsNavigationProps { * Menu items are shown based on the permissions. If organizations can be * viewed, groups are skipped since they will show under each org instead. */ -const DeploymentSettingsNavigation: FC = ( - props, -) => { +const DeploymentSettingsNavigation: FC = ({ + active, + permissions, +}) => { return (
= ( > Deployment - {props.active && ( + {active && ( - {props.permissions.viewDeploymentValues && ( + {permissions.viewDeploymentValues && ( General )} - {props.permissions.viewAllLicenses && ( + {permissions.viewAllLicenses && ( Licenses )} - {props.permissions.editDeploymentValues && ( + {permissions.editDeploymentValues && ( Appearance )} - {props.permissions.viewDeploymentValues && ( + {permissions.viewDeploymentValues && ( User Authentication )} - {props.permissions.viewDeploymentValues && ( + {permissions.viewDeploymentValues && ( External Authentication @@ -102,27 +112,27 @@ const DeploymentSettingsNavigation: FC = ( Network )} {/* All users can view workspace regions. */} Workspace Proxies - {props.permissions.viewDeploymentValues && ( + {permissions.viewDeploymentValues && ( Security )} - {props.permissions.viewDeploymentValues && ( + {permissions.viewDeploymentValues && ( Observability )} - {props.permissions.viewAllUsers && ( + {permissions.viewAllUsers && ( Users )} - {props.permissions.viewAnyAuditLog && ( + {permissions.viewAnyAuditLog && ( Auditing @@ -156,23 +166,20 @@ interface OrganizationsSettingsNavigationProps { */ const OrganizationsSettingsNavigation: FC< OrganizationsSettingsNavigationProps -> = (props) => { +> = ({ activeOrganizationName, organizations, permissions }) => { // Wait for organizations and their permissions to load in. - if (!props.organizations) { + if (!organizations) { return ; } - if ( - props.organizations.length <= 0 && - !props.permissions.createOrganization - ) { + if (organizations.length <= 0 && !permissions.createOrganization) { return null; } return ( <>
Organizations
- {props.permissions.createOrganization && ( + {permissions.createOrganization && ( )} - {props.organizations.map((org) => ( + {organizations.map((org) => ( ))} @@ -208,43 +215,40 @@ interface OrganizationSettingsNavigationProps { */ const OrganizationSettingsNavigation: FC< OrganizationSettingsNavigationProps -> = (props) => { +> = ({ active, organization }) => { return ( <> } > - {props.organization.display_name} + {organization.display_name} - {props.active && ( + {active && ( - {props.organization.permissions.editOrganization && ( - + {organization.permissions.editOrganization && ( + Organization settings )} - {props.organization.permissions.editMembers && ( + {organization.permissions.editMembers && ( Members )} - {props.organization.permissions.editGroups && ( + {organization.permissions.editGroups && ( Groups @@ -252,11 +256,11 @@ const OrganizationSettingsNavigation: FC< {/* For now redirect to the site-wide audit page with the organization pre-filled into the filter. Based on user feedback we might want to serve a copy of the audit page or even delete this link. */} - {props.organization.permissions.auditOrganization && ( + {organization.permissions.auditOrganization && ( Auditing From 02e7e47b70eaac8a25f3e0cb4fbde2cca9154853 Mon Sep 17 00:00:00 2001 From: Asher Date: Thu, 8 Aug 2024 14:20:00 -0800 Subject: [PATCH 10/13] No need for extra routes with a bit of ? --- .../OrganizationSettingsPage.test.tsx | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/site/src/pages/ManagementSettingsPage/OrganizationSettingsPage.test.tsx b/site/src/pages/ManagementSettingsPage/OrganizationSettingsPage.test.tsx index a32682a33aac3..e6107629920a4 100644 --- a/site/src/pages/ManagementSettingsPage/OrganizationSettingsPage.test.tsx +++ b/site/src/pages/ManagementSettingsPage/OrganizationSettingsPage.test.tsx @@ -16,13 +16,7 @@ jest.spyOn(console, "error").mockImplementation(() => {}); const renderRootPage = async () => { renderWithManagementSettingsLayout(, { route: "/organizations", - path: "/organizations", - extraRoutes: [ - { - path: "/organizations/:organization", - element: , - }, - ], + path: "/organizations/:organization?", }); await waitForLoaderToBeRemoved(); }; From ddd2ea54a0259c465b072bec535c8c70204a18b2 Mon Sep 17 00:00:00 2001 From: Asher Date: Thu, 8 Aug 2024 14:38:17 -0800 Subject: [PATCH 11/13] Add org IDs to permissions query --- site/src/api/queries/organizations.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/site/src/api/queries/organizations.ts b/site/src/api/queries/organizations.ts index 05ab5d40d6ce0..1903e73397d9c 100644 --- a/site/src/api/queries/organizations.ts +++ b/site/src/api/queries/organizations.ts @@ -175,7 +175,7 @@ export const organizationsPermissions = ( } return { - queryKey: ["organizations", "permissions"], + queryKey: ["organizations", organizationIds.sort(), "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) From 64631e1448f2c16d8fb8eb4b8c7080a55e5fbe00 Mon Sep 17 00:00:00 2001 From: Asher Date: Thu, 8 Aug 2024 14:48:37 -0800 Subject: [PATCH 12/13] Two is now one --- .../ManagementSettingsLayout.tsx | 2 +- site/src/pages/ManagementSettingsPage/Sidebar.tsx | 11 ++++------- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/site/src/pages/ManagementSettingsPage/ManagementSettingsLayout.tsx b/site/src/pages/ManagementSettingsPage/ManagementSettingsLayout.tsx index 8fb08330e5f73..c31bbfe2b54c7 100644 --- a/site/src/pages/ManagementSettingsPage/ManagementSettingsLayout.tsx +++ b/site/src/pages/ManagementSettingsPage/ManagementSettingsLayout.tsx @@ -28,7 +28,7 @@ export const canEditOrganization = ( permissions: AuthorizationResponse | undefined, ) => { return ( - permissions && + permissions !== undefined && (permissions.editOrganization || permissions.editMembers || permissions.editGroups) diff --git a/site/src/pages/ManagementSettingsPage/Sidebar.tsx b/site/src/pages/ManagementSettingsPage/Sidebar.tsx index be870d7791d85..6ac55c59b999f 100644 --- a/site/src/pages/ManagementSettingsPage/Sidebar.tsx +++ b/site/src/pages/ManagementSettingsPage/Sidebar.tsx @@ -38,13 +38,10 @@ export const Sidebar: FC = () => { permissions: orgPermissionsQuery.data?.[org.id], }; }) - // TypeScript is not able to infer whether permissions are defined from the - // canEditOrganization call, so although redundant this helps figure it out. - .filter( - (org): org is OrganizationWithPermissions => - org.permissions !== undefined, - ) - .filter((org) => { + // TypeScript is not able to infer whether permissions are defined on the + // object even if we explicitly check org.permissions here, so add the `is` + // here to help out (canEditOrganization does the actual check). + .filter((org): org is OrganizationWithPermissions => { return canEditOrganization(org.permissions); }); From ba22bc96dc4f38b9e5c3c5173d582ecd0695cde4 Mon Sep 17 00:00:00 2001 From: Asher Date: Thu, 8 Aug 2024 22:03:59 -0800 Subject: [PATCH 13/13] fixup! Merge remote-tracking branch 'github/main' into asher/show-editable-orgs --- site/src/api/queries/organizations.ts | 13 ++++++++++--- .../pages/ManagementSettingsPage/SidebarView.tsx | 4 ++-- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/site/src/api/queries/organizations.ts b/site/src/api/queries/organizations.ts index 83a358ef97f31..0f3a7ae8c3c01 100644 --- a/site/src/api/queries/organizations.ts +++ b/site/src/api/queries/organizations.ts @@ -185,9 +185,9 @@ export const organizationsPermissions = ( queryKey: ["organizations", organizationIds.sort(), "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). + // per sub-link (audit, settings, groups, roles, and members pages) 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: { @@ -217,6 +217,13 @@ export const organizationsPermissions = ( }, action: "read", }, + assignOrgRole: { + object: { + resource_type: "assign_org_role", + organization_id: organizationId, + }, + action: "create", + }, }); // The endpoint takes a flat array, so to avoid collisions prepend each diff --git a/site/src/pages/ManagementSettingsPage/SidebarView.tsx b/site/src/pages/ManagementSettingsPage/SidebarView.tsx index 908cc29c038fe..f5a31e2bce514 100644 --- a/site/src/pages/ManagementSettingsPage/SidebarView.tsx +++ b/site/src/pages/ManagementSettingsPage/SidebarView.tsx @@ -256,10 +256,10 @@ const OrganizationSettingsNavigation: FC< Groups )} - {props.permissions.assignOrgRole && + {organization.permissions.assignOrgRole && experiments.includes("custom-roles") && ( Roles