From 1ad09fa16589cede3b16398b06b9b12dd2e3e409 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Thu, 24 Oct 2024 19:42:16 +0000 Subject: [PATCH 01/30] refactor: update layout so that sidebar always shows --- site/src/@types/storybook.d.ts | 1 + site/src/contexts/auth/permissions.tsx | 7 +- .../modules/dashboard/DashboardProvider.tsx | 8 +++ .../management/ManagementSettingsLayout.tsx | 68 ++++++++----------- site/src/modules/management/Sidebar.tsx | 12 +--- .../CustomRolesPage/CreateEditRolePage.tsx | 10 +-- .../CustomRolesPage/CustomRolesPage.tsx | 9 +-- .../GroupsPage/GroupsPage.tsx | 11 +-- .../IdpSyncPage/IdpSyncPage.tsx | 9 ++- .../OrganizationMembersPage.tsx | 9 +-- .../OrganizationProvisionersPage.tsx | 3 +- .../OrganizationSettingsPage.tsx | 17 ++--- .../OrganizationSettingsPageView.stories.tsx | 1 - .../WorkspacePage/WorkspacePage.test.tsx | 1 + .../WorkspacesPageView.stories.tsx | 16 +++-- site/src/testHelpers/storybook.tsx | 8 +-- 16 files changed, 92 insertions(+), 98 deletions(-) diff --git a/site/src/@types/storybook.d.ts b/site/src/@types/storybook.d.ts index 82507741d5621..e572f7b385ecf 100644 --- a/site/src/@types/storybook.d.ts +++ b/site/src/@types/storybook.d.ts @@ -19,6 +19,7 @@ declare module "@storybook/react" { experiments?: Experiments; showOrganizations?: boolean; organizations?: Organization[]; + activeOrganization?: Organization; queries?: { key: QueryKey; data: unknown; isError?: boolean }[]; webSocket?: WebSocketEvent[]; user?: User; diff --git a/site/src/contexts/auth/permissions.tsx b/site/src/contexts/auth/permissions.tsx index 0c89d81686d2f..e6e6ef41ebd02 100644 --- a/site/src/contexts/auth/permissions.tsx +++ b/site/src/contexts/auth/permissions.tsx @@ -1,3 +1,5 @@ +import type { AuthorizationCheck } from "api/typesGenerated"; + export const checks = { viewAllUsers: "viewAllUsers", updateUsers: "updateUsers", @@ -17,7 +19,7 @@ export const checks = { viewAnyGroup: "viewAnyGroup", createGroup: "createGroup", viewAllLicenses: "viewAllLicenses", -} as const; +} as const satisfies Record; export const permissionsToCheck = { [checks.viewAllUsers]: { @@ -116,7 +118,6 @@ export const permissionsToCheck = { [checks.viewAnyGroup]: { object: { resource_type: "group", - org_id: "any", }, action: "read", }, @@ -132,6 +133,6 @@ export const permissionsToCheck = { }, action: "read", }, -} as const; +} as const satisfies Record; export type Permissions = Record; diff --git a/site/src/modules/dashboard/DashboardProvider.tsx b/site/src/modules/dashboard/DashboardProvider.tsx index d8fa339deccbb..52dc8dba346b2 100644 --- a/site/src/modules/dashboard/DashboardProvider.tsx +++ b/site/src/modules/dashboard/DashboardProvider.tsx @@ -14,12 +14,14 @@ import { useEmbeddedMetadata } from "hooks/useEmbeddedMetadata"; import { type FC, type PropsWithChildren, createContext } from "react"; import { useQuery } from "react-query"; import { selectFeatureVisibility } from "./entitlements"; +import { useParams } from "react-router-dom"; export interface DashboardValue { entitlements: Entitlements; experiments: Experiments; appearance: AppearanceConfig; organizations: readonly Organization[]; + activeOrganization: Organization | undefined; showOrganizations: boolean; } @@ -33,6 +35,9 @@ export const DashboardProvider: FC = ({ children }) => { const experimentsQuery = useQuery(experiments(metadata.experiments)); const appearanceQuery = useQuery(appearance(metadata.appearance)); const organizationsQuery = useQuery(organizations()); + const { organization: organizationName } = useParams() as { + organization?: string; + }; const error = entitlementsQuery.error || @@ -67,6 +72,9 @@ export const DashboardProvider: FC = ({ children }) => { appearance: appearanceQuery.data, organizations: organizationsQuery.data, showOrganizations: hasMultipleOrganizations || organizationsEnabled, + activeOrganization: organizationsQuery.data?.find( + (org) => org.name === organizationName, + ), }} > {children} diff --git a/site/src/modules/management/ManagementSettingsLayout.tsx b/site/src/modules/management/ManagementSettingsLayout.tsx index b9fcbc0936e4b..ffa594a77923d 100644 --- a/site/src/modules/management/ManagementSettingsLayout.tsx +++ b/site/src/modules/management/ManagementSettingsLayout.tsx @@ -13,16 +13,14 @@ import { useQuery } from "react-query"; import { Outlet, useParams } from "react-router-dom"; import { Sidebar } from "./Sidebar"; -export const ManagementSettingsContext = createContext< - ManagementSettingsValue | undefined ->(undefined); - type ManagementSettingsValue = Readonly<{ deploymentValues: DeploymentConfig; - organizations: readonly Organization[]; - organization?: Organization; }>; +export const ManagementSettingsContext = createContext< + ManagementSettingsValue | undefined +>(undefined); + export const useManagementSettings = (): ManagementSettingsValue => { const context = useContext(ManagementSettingsContext); if (!context) { @@ -56,11 +54,6 @@ export const canEditOrganization = ( */ export const ManagementSettingsLayout: FC = () => { const { permissions } = useAuthenticated(); - const deploymentConfigQuery = useQuery(deploymentConfig()); - const { organizations } = useDashboard(); - const { organization: orgName } = useParams() as { - organization?: string; - }; // The deployment settings page also contains users, audit logs, groups and // organizations, so this page must be visible if you can see any of these. @@ -70,39 +63,38 @@ export const ManagementSettingsLayout: FC = () => { permissions.editAnyOrganization || permissions.viewAnyAuditLog; - if (deploymentConfigQuery.error) { - return ; - } + const deploymentConfigQuery = useQuery({ + ...deploymentConfig(), + enabled: canViewDeploymentSettingsPage, + }); - if (!deploymentConfigQuery.data) { + if (deploymentConfigQuery.isLoading) { return ; } - const organization = - organizations && orgName - ? organizations.find((org) => org.name === orgName) - : undefined; - return ( - - - - -
- }> - - -
- - - + + + + +
+ {deploymentConfigQuery.isError ? ( + + ) : ( + + }> + + + + )} +
+ + ); }; diff --git a/site/src/modules/management/Sidebar.tsx b/site/src/modules/management/Sidebar.tsx index a2560fe5d6515..597ca1c5ec3ad 100644 --- a/site/src/modules/management/Sidebar.tsx +++ b/site/src/modules/management/Sidebar.tsx @@ -1,10 +1,7 @@ import { organizationsPermissions } from "api/queries/organizations"; import { useAuthenticated } from "contexts/auth/RequireAuth"; import { useDashboard } from "modules/dashboard/useDashboard"; -import { - canEditOrganization, - useManagementSettings, -} from "modules/management/ManagementSettingsLayout"; +import { canEditOrganization } from "modules/management/ManagementSettingsLayout"; import type { FC } from "react"; import { useQuery } from "react-query"; import { useLocation, useParams } from "react-router-dom"; @@ -20,10 +17,7 @@ import { type OrganizationWithPermissions, SidebarView } from "./SidebarView"; export const Sidebar: FC = () => { const location = useLocation(); const { permissions } = useAuthenticated(); - const { organizations } = useManagementSettings(); - const { organization: organizationName } = useParams() as { - organization?: string; - }; + const { organizations, activeOrganization } = useDashboard(); const orgPermissionsQuery = useQuery( organizationsPermissions(organizations?.map((o) => o.id)), @@ -52,7 +46,7 @@ export const Sidebar: FC = () => { // the user is on /organizations but has no editable organizations to // which we can redirect. activeSettings={location.pathname.startsWith("/deployment")} - activeOrganizationName={organizationName} + activeOrganizationName={activeOrganization?.name} organizations={editableOrgs} permissions={permissions} /> diff --git a/site/src/pages/ManagementSettingsPage/CustomRolesPage/CreateEditRolePage.tsx b/site/src/pages/ManagementSettingsPage/CustomRolesPage/CreateEditRolePage.tsx index 80995a160b67d..c46dc4c2b1c9b 100644 --- a/site/src/pages/ManagementSettingsPage/CustomRolesPage/CreateEditRolePage.tsx +++ b/site/src/pages/ManagementSettingsPage/CustomRolesPage/CreateEditRolePage.tsx @@ -8,24 +8,26 @@ import { import type { CustomRoleRequest } from "api/typesGenerated"; import { displayError } from "components/GlobalSnackbar/utils"; import { Loader } from "components/Loader/Loader"; -import { useManagementSettings } from "modules/management/ManagementSettingsLayout"; import type { FC } from "react"; import { Helmet } from "react-helmet-async"; import { useMutation, useQuery, useQueryClient } from "react-query"; import { useNavigate, useParams } from "react-router-dom"; import { pageTitle } from "utils/page"; import CreateEditRolePageView from "./CreateEditRolePageView"; +import { useDashboard } from "modules/dashboard/useDashboard"; export const CreateEditRolePage: FC = () => { const queryClient = useQueryClient(); const navigate = useNavigate(); + const { activeOrganization } = useDashboard(); const { organization: organizationName, roleName } = useParams() as { organization: string; roleName: string; }; - const { organizations } = useManagementSettings(); - const organization = organizations?.find((o) => o.name === organizationName); - const permissionsQuery = useQuery(organizationPermissions(organization?.id)); + + const permissionsQuery = useQuery( + organizationPermissions(activeOrganization?.id), + ); const createOrganizationRoleMutation = useMutation( createOrganizationRole(queryClient, organizationName), ); diff --git a/site/src/pages/ManagementSettingsPage/CustomRolesPage/CustomRolesPage.tsx b/site/src/pages/ManagementSettingsPage/CustomRolesPage/CustomRolesPage.tsx index 34c33083a76be..1d78e9bf79e08 100644 --- a/site/src/pages/ManagementSettingsPage/CustomRolesPage/CustomRolesPage.tsx +++ b/site/src/pages/ManagementSettingsPage/CustomRolesPage/CustomRolesPage.tsx @@ -8,13 +8,13 @@ import { Loader } from "components/Loader/Loader"; import { SettingsHeader } from "components/SettingsHeader/SettingsHeader"; import { Stack } from "components/Stack/Stack"; import { useFeatureVisibility } from "modules/dashboard/useFeatureVisibility"; -import { useManagementSettings } from "modules/management/ManagementSettingsLayout"; import { type FC, useEffect, useState } from "react"; import { Helmet } from "react-helmet-async"; import { useMutation, useQuery, useQueryClient } from "react-query"; import { useParams } from "react-router-dom"; import { pageTitle } from "utils/page"; import CustomRolesPageView from "./CustomRolesPageView"; +import { useDashboard } from "modules/dashboard/useDashboard"; export const CustomRolesPage: FC = () => { const queryClient = useQueryClient(); @@ -22,9 +22,10 @@ export const CustomRolesPage: FC = () => { const { organization: organizationName } = useParams() as { organization: string; }; - const { organizations } = useManagementSettings(); - const organization = organizations?.find((o) => o.name === organizationName); - const permissionsQuery = useQuery(organizationPermissions(organization?.id)); + const { activeOrganization } = useDashboard(); + const permissionsQuery = useQuery( + organizationPermissions(activeOrganization?.id), + ); const deleteRoleMutation = useMutation( deleteOrganizationRole(queryClient, organizationName), ); diff --git a/site/src/pages/ManagementSettingsPage/GroupsPage/GroupsPage.tsx b/site/src/pages/ManagementSettingsPage/GroupsPage/GroupsPage.tsx index 774360dc6a6d1..9d2f20f9d3b07 100644 --- a/site/src/pages/ManagementSettingsPage/GroupsPage/GroupsPage.tsx +++ b/site/src/pages/ManagementSettingsPage/GroupsPage/GroupsPage.tsx @@ -10,13 +10,13 @@ import { Loader } from "components/Loader/Loader"; import { SettingsHeader } from "components/SettingsHeader/SettingsHeader"; import { Stack } from "components/Stack/Stack"; import { useFeatureVisibility } from "modules/dashboard/useFeatureVisibility"; -import { useManagementSettings } from "modules/management/ManagementSettingsLayout"; import { type FC, useEffect } from "react"; import { Helmet } from "react-helmet-async"; import { useQuery } from "react-query"; import { Navigate, Link as RouterLink, useParams } from "react-router-dom"; import { pageTitle } from "utils/page"; import GroupsPageView from "./GroupsPageView"; +import { useDashboard } from "modules/dashboard/useDashboard"; export const GroupsPage: FC = () => { const feats = useFeatureVisibility(); @@ -24,9 +24,10 @@ export const GroupsPage: FC = () => { organization: string; }; const groupsQuery = useQuery(groupsByOrganization(organizationName)); - const { organizations } = useManagementSettings(); - const organization = organizations?.find((o) => o.name === organizationName); - const permissionsQuery = useQuery(organizationPermissions(organization?.id)); + const { organizations, activeOrganization } = useDashboard(); + const permissionsQuery = useQuery( + organizationPermissions(activeOrganization?.id), + ); useEffect(() => { if (groupsQuery.error) { @@ -57,7 +58,7 @@ export const GroupsPage: FC = () => { throw new Error("No default organization found"); } - if (!organization) { + if (!activeOrganization) { return ; } diff --git a/site/src/pages/ManagementSettingsPage/IdpSyncPage/IdpSyncPage.tsx b/site/src/pages/ManagementSettingsPage/IdpSyncPage/IdpSyncPage.tsx index ef432e8b0d6d6..9c3b908bd7cb9 100644 --- a/site/src/pages/ManagementSettingsPage/IdpSyncPage/IdpSyncPage.tsx +++ b/site/src/pages/ManagementSettingsPage/IdpSyncPage/IdpSyncPage.tsx @@ -11,7 +11,6 @@ import { Paywall } from "components/Paywall/Paywall"; import { SettingsHeader } from "components/SettingsHeader/SettingsHeader"; import { Stack } from "components/Stack/Stack"; import { useFeatureVisibility } from "modules/dashboard/useFeatureVisibility"; -import { useManagementSettings } from "modules/management/ManagementSettingsLayout"; import type { FC } from "react"; import { Helmet } from "react-helmet-async"; import { useQueries } from "react-query"; @@ -20,6 +19,7 @@ import { docs } from "utils/docs"; import { pageTitle } from "utils/page"; import { IdpSyncHelpTooltip } from "./IdpSyncHelpTooltip"; import IdpSyncPageView from "./IdpSyncPageView"; +import { useDashboard } from "modules/dashboard/useDashboard"; export const IdpSyncPage: FC = () => { const { organization: organizationName } = useParams() as { @@ -27,8 +27,7 @@ export const IdpSyncPage: FC = () => { }; // IdP sync does not have its own entitlement and is based on templace_rbac const { template_rbac: isIdpSyncEnabled } = useFeatureVisibility(); - const { organizations } = useManagementSettings(); - const organization = organizations?.find((o) => o.name === organizationName); + const { activeOrganization } = useDashboard(); const [groupIdpSyncSettingsQuery, roleIdpSyncSettingsQuery, groupsQuery] = useQueries({ @@ -39,7 +38,7 @@ export const IdpSyncPage: FC = () => { ], }); - if (!organization) { + if (!activeOrganization) { return ; } @@ -94,7 +93,7 @@ export const IdpSyncPage: FC = () => { roleSyncSettings={roleIdpSyncSettingsQuery.data} groups={groupsQuery.data} groupsMap={groupsMap} - organization={organization} + organization={activeOrganization} error={error} /> diff --git a/site/src/pages/ManagementSettingsPage/OrganizationMembersPage.tsx b/site/src/pages/ManagementSettingsPage/OrganizationMembersPage.tsx index de6295a838bd9..22e80901cc9bf 100644 --- a/site/src/pages/ManagementSettingsPage/OrganizationMembersPage.tsx +++ b/site/src/pages/ManagementSettingsPage/OrganizationMembersPage.tsx @@ -15,11 +15,11 @@ import { displayError, displaySuccess } from "components/GlobalSnackbar/utils"; import { Loader } from "components/Loader/Loader"; import { Stack } from "components/Stack/Stack"; import { useAuthenticated } from "contexts/auth/RequireAuth"; -import { useManagementSettings } from "modules/management/ManagementSettingsLayout"; import { type FC, useState } from "react"; import { useMutation, useQuery, useQueryClient } from "react-query"; import { useParams } from "react-router-dom"; import { OrganizationMembersPageView } from "./OrganizationMembersPageView"; +import { useDashboard } from "modules/dashboard/useDashboard"; const OrganizationMembersPage: FC = () => { const queryClient = useQueryClient(); @@ -50,9 +50,10 @@ const OrganizationMembersPage: FC = () => { updateOrganizationMemberRoles(queryClient, organizationName), ); - const { organizations } = useManagementSettings(); - const organization = organizations?.find((o) => o.name === organizationName); - const permissionsQuery = useQuery(organizationPermissions(organization?.id)); + const { activeOrganization } = useDashboard(); + const permissionsQuery = useQuery( + organizationPermissions(activeOrganization?.id), + ); const [memberToDelete, setMemberToDelete] = useState(); diff --git a/site/src/pages/ManagementSettingsPage/OrganizationProvisionersPage.tsx b/site/src/pages/ManagementSettingsPage/OrganizationProvisionersPage.tsx index bd91c348e03ee..26d518584aa9f 100644 --- a/site/src/pages/ManagementSettingsPage/OrganizationProvisionersPage.tsx +++ b/site/src/pages/ManagementSettingsPage/OrganizationProvisionersPage.tsx @@ -4,7 +4,6 @@ import type { Organization } from "api/typesGenerated"; import { EmptyState } from "components/EmptyState/EmptyState"; import { useEmbeddedMetadata } from "hooks/useEmbeddedMetadata"; import { useDashboard } from "modules/dashboard/useDashboard"; -import { useManagementSettings } from "modules/management/ManagementSettingsLayout"; import type { FC } from "react"; import { useQuery } from "react-query"; import { useParams } from "react-router-dom"; @@ -14,7 +13,7 @@ const OrganizationProvisionersPage: FC = () => { const { organization: organizationName } = useParams() as { organization: string; }; - const { organizations } = useManagementSettings(); + const { organizations } = useDashboard(); const { entitlements } = useDashboard(); const { metadata } = useEmbeddedMetadata(); diff --git a/site/src/pages/ManagementSettingsPage/OrganizationSettingsPage.tsx b/site/src/pages/ManagementSettingsPage/OrganizationSettingsPage.tsx index 1d11c85a605ae..2a44dad3a3f19 100644 --- a/site/src/pages/ManagementSettingsPage/OrganizationSettingsPage.tsx +++ b/site/src/pages/ManagementSettingsPage/OrganizationSettingsPage.tsx @@ -9,21 +9,16 @@ import { EmptyState } from "components/EmptyState/EmptyState"; import { displaySuccess } from "components/GlobalSnackbar/utils"; import { Loader } from "components/Loader/Loader"; import { useFeatureVisibility } from "modules/dashboard/useFeatureVisibility"; -import { - canEditOrganization, - useManagementSettings, -} from "modules/management/ManagementSettingsLayout"; +import { canEditOrganization } from "modules/management/ManagementSettingsLayout"; import type { FC } from "react"; import { useMutation, useQuery, useQueryClient } from "react-query"; import { Navigate, useNavigate, useParams } from "react-router-dom"; import { OrganizationSettingsPageView } from "./OrganizationSettingsPageView"; import { OrganizationSummaryPageView } from "./OrganizationSummaryPageView"; +import { useDashboard } from "modules/dashboard/useDashboard"; const OrganizationSettingsPage: FC = () => { - const { organization: organizationName } = useParams() as { - organization?: string; - }; - const { organizations } = useManagementSettings(); + const { organizations, activeOrganization } = useDashboard(); const feats = useFeatureVisibility(); const navigate = useNavigate(); @@ -36,8 +31,8 @@ const OrganizationSettingsPage: FC = () => { ); const organization = - organizations && organizationName - ? getOrganizationByName(organizations, organizationName) + organizations && activeOrganization + ? getOrganizationByName(organizations, activeOrganization.name) : undefined; const permissionsQuery = useQuery( organizationsPermissions(organizations?.map((o) => o.id)), @@ -54,7 +49,7 @@ const OrganizationSettingsPage: FC = () => { // Redirect /organizations => /organizations/default-org, or if they cannot edit // the default org, then the first org they can edit, if any. - if (!organizationName) { + if (!activeOrganization?.name) { const editableOrg = [...organizations] .sort((a, b) => { // Prefer default org (it may not be first). diff --git a/site/src/pages/ManagementSettingsPage/OrganizationSettingsPageView.stories.tsx b/site/src/pages/ManagementSettingsPage/OrganizationSettingsPageView.stories.tsx index 9983c25080a59..3e8b1ad3133b7 100644 --- a/site/src/pages/ManagementSettingsPage/OrganizationSettingsPageView.stories.tsx +++ b/site/src/pages/ManagementSettingsPage/OrganizationSettingsPageView.stories.tsx @@ -4,7 +4,6 @@ import { MockDefaultOrganization, MockOrganization, } from "testHelpers/entities"; -import { withManagementSettingsProvider } from "testHelpers/storybook"; import { OrganizationSettingsPageView } from "./OrganizationSettingsPageView"; const meta: Meta = { diff --git a/site/src/pages/WorkspacePage/WorkspacePage.test.tsx b/site/src/pages/WorkspacePage/WorkspacePage.test.tsx index 2c113b933483e..0ec52f7bd3768 100644 --- a/site/src/pages/WorkspacePage/WorkspacePage.test.tsx +++ b/site/src/pages/WorkspacePage/WorkspacePage.test.tsx @@ -562,6 +562,7 @@ describe("WorkspacePage", () => { entitlements: MockEntitlements, experiments: [], organizations: [MockOrganization], + activeOrganization: MockOrganization, showOrganizations: true, }} > diff --git a/site/src/pages/WorkspacesPage/WorkspacesPageView.stories.tsx b/site/src/pages/WorkspacesPage/WorkspacesPageView.stories.tsx index ef639d087fb5a..c74a1f708cc28 100644 --- a/site/src/pages/WorkspacesPage/WorkspacesPageView.stories.tsx +++ b/site/src/pages/WorkspacesPage/WorkspacesPageView.stories.tsx @@ -1,6 +1,7 @@ import type { Meta, StoryObj } from "@storybook/react"; import { expect, within } from "@storybook/test"; import { + type Organization, type Workspace, type WorkspaceStatus, WorkspaceStatuses, @@ -269,6 +270,12 @@ export const InvalidPageNumber: Story = { }, }; +const mockOrganization: Organization = { + ...MockOrganization, + name: "limbus-co", + display_name: "Limbus Company, LLC", +}; + export const ShowOrganizations: Story = { args: { workspaces: [{ ...MockWorkspace, organization_name: "limbus-co" }], @@ -276,13 +283,8 @@ export const ShowOrganizations: Story = { parameters: { showOrganizations: true, - organizations: [ - { - ...MockOrganization, - name: "limbus-co", - display_name: "Limbus Company, LLC", - }, - ], + activeOrganization: mockOrganization, + organizations: [mockOrganization], }, play: async ({ canvasElement }) => { diff --git a/site/src/testHelpers/storybook.tsx b/site/src/testHelpers/storybook.tsx index a76e1230205fc..cae8e7714ae62 100644 --- a/site/src/testHelpers/storybook.tsx +++ b/site/src/testHelpers/storybook.tsx @@ -26,6 +26,7 @@ export const withDashboardProvider = ( experiments = [], showOrganizations = false, organizations = [MockDefaultOrganization], + activeOrganization = organizations[0] ?? MockDefaultOrganization, } = parameters; const entitlements: Entitlements = { @@ -48,6 +49,7 @@ export const withDashboardProvider = ( experiments, organizations, showOrganizations, + activeOrganization, appearance: MockAppearanceConfig, }} > @@ -130,11 +132,7 @@ export const withGlobalSnackbar = (Story: FC) => ( export const withManagementSettingsProvider = (Story: FC) => { return ( From 620bd3a758307f83891d016bf797453f733327be Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Thu, 24 Oct 2024 20:03:29 +0000 Subject: [PATCH 02/30] refactor: consolidate logic more --- .../OrganizationProvisionersPage.tsx | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/site/src/pages/ManagementSettingsPage/OrganizationProvisionersPage.tsx b/site/src/pages/ManagementSettingsPage/OrganizationProvisionersPage.tsx index 26d518584aa9f..62c5a60f01e9f 100644 --- a/site/src/pages/ManagementSettingsPage/OrganizationProvisionersPage.tsx +++ b/site/src/pages/ManagementSettingsPage/OrganizationProvisionersPage.tsx @@ -13,18 +13,14 @@ const OrganizationProvisionersPage: FC = () => { const { organization: organizationName } = useParams() as { organization: string; }; - const { organizations } = useDashboard(); + const { activeOrganization } = useDashboard(); const { entitlements } = useDashboard(); const { metadata } = useEmbeddedMetadata(); const buildInfoQuery = useQuery(buildInfo(metadata["build-info"])); - - const organization = organizations - ? getOrganizationByName(organizations, organizationName) - : undefined; const provisionersQuery = useQuery(provisionerDaemonGroups(organizationName)); - if (!organization) { + if (!activeOrganization) { return ; } From 7ee3cf02563a7fddce55d89e85fcd83ff9625d20 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Thu, 24 Oct 2024 20:25:47 +0000 Subject: [PATCH 03/30] fix: add redirect for base users --- site/src/modules/management/ManagementSettingsLayout.tsx | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/site/src/modules/management/ManagementSettingsLayout.tsx b/site/src/modules/management/ManagementSettingsLayout.tsx index ffa594a77923d..1ab3ac8953db6 100644 --- a/site/src/modules/management/ManagementSettingsLayout.tsx +++ b/site/src/modules/management/ManagementSettingsLayout.tsx @@ -10,7 +10,7 @@ import { RequirePermission } from "contexts/auth/RequirePermission"; import { useDashboard } from "modules/dashboard/useDashboard"; import { type FC, Suspense, createContext, useContext } from "react"; import { useQuery } from "react-query"; -import { Outlet, useParams } from "react-router-dom"; +import { Navigate, Outlet, useParams } from "react-router-dom"; import { Sidebar } from "./Sidebar"; type ManagementSettingsValue = Readonly<{ @@ -68,6 +68,13 @@ export const ManagementSettingsLayout: FC = () => { enabled: canViewDeploymentSettingsPage, }); + // Need to make sure that we redirect basic users. If the enabled value for + // deploymentConfigQuery is false, the query object will be stuck in a + // loading state forever + if (!canViewDeploymentSettingsPage) { + return ; + } + if (deploymentConfigQuery.isLoading) { return ; } From e889621fd058d3f433836b2238630e1f7bd20d1e Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Thu, 24 Oct 2024 21:47:40 +0000 Subject: [PATCH 04/30] fix: update routing logic to not block on disabled queries --- site/src/contexts/auth/permissions.tsx | 14 +++++ .../management/ManagementSettingsLayout.tsx | 62 +++++++++---------- .../management/SidebarView.stories.tsx | 5 +- site/src/modules/management/SidebarView.tsx | 37 ++++++----- site/src/testHelpers/entities.ts | 25 ++++++++ 5 files changed, 87 insertions(+), 56 deletions(-) diff --git a/site/src/contexts/auth/permissions.tsx b/site/src/contexts/auth/permissions.tsx index e6e6ef41ebd02..b8996a623ea02 100644 --- a/site/src/contexts/auth/permissions.tsx +++ b/site/src/contexts/auth/permissions.tsx @@ -13,12 +13,14 @@ export const checks = { viewUpdateCheck: "viewUpdateCheck", viewExternalAuthConfig: "viewExternalAuthConfig", viewDeploymentStats: "viewDeploymentStats", + readWorkspaceProxies: "readWorkspaceProxies", editWorkspaceProxies: "editWorkspaceProxies", createOrganization: "createOrganization", editAnyOrganization: "editAnyOrganization", viewAnyGroup: "viewAnyGroup", createGroup: "createGroup", viewAllLicenses: "viewAllLicenses", + viewNotificationTemplate: "viewNotificationTemplate", } as const satisfies Record; export const permissionsToCheck = { @@ -96,6 +98,12 @@ export const permissionsToCheck = { }, action: "read", }, + [checks.readWorkspaceProxies]: { + object: { + resource_type: "workspace_proxy", + }, + action: "read", + }, [checks.editWorkspaceProxies]: { object: { resource_type: "workspace_proxy", @@ -133,6 +141,12 @@ export const permissionsToCheck = { }, action: "read", }, + [checks.viewNotificationTemplate]: { + object: { + resource_type: "notification_template", + }, + action: "read", + }, } as const satisfies Record; export type Permissions = Record; diff --git a/site/src/modules/management/ManagementSettingsLayout.tsx b/site/src/modules/management/ManagementSettingsLayout.tsx index 1ab3ac8953db6..920b075f94a5f 100644 --- a/site/src/modules/management/ManagementSettingsLayout.tsx +++ b/site/src/modules/management/ManagementSettingsLayout.tsx @@ -1,20 +1,19 @@ import type { DeploymentConfig } from "api/api"; import { deploymentConfig } from "api/queries/deployment"; -import type { AuthorizationResponse, Organization } from "api/typesGenerated"; +import type { AuthorizationResponse } from "api/typesGenerated"; import { ErrorAlert } from "components/Alert/ErrorAlert"; import { Loader } from "components/Loader/Loader"; import { Margins } from "components/Margins/Margins"; import { Stack } from "components/Stack/Stack"; import { useAuthenticated } from "contexts/auth/RequireAuth"; import { RequirePermission } from "contexts/auth/RequirePermission"; -import { useDashboard } from "modules/dashboard/useDashboard"; import { type FC, Suspense, createContext, useContext } from "react"; import { useQuery } from "react-query"; -import { Navigate, Outlet, useParams } from "react-router-dom"; +import { Outlet } from "react-router-dom"; import { Sidebar } from "./Sidebar"; type ManagementSettingsValue = Readonly<{ - deploymentValues: DeploymentConfig; + deploymentValues: DeploymentConfig | undefined; }>; export const ManagementSettingsContext = createContext< @@ -54,51 +53,46 @@ export const canEditOrganization = ( */ export const ManagementSettingsLayout: FC = () => { const { permissions } = useAuthenticated(); - - // The deployment settings page also contains users, audit logs, groups and - // organizations, so this page must be visible if you can see any of these. - const canViewDeploymentSettingsPage = - permissions.viewDeploymentValues || - permissions.viewAllUsers || - permissions.editAnyOrganization || - permissions.viewAnyAuditLog; - const deploymentConfigQuery = useQuery({ ...deploymentConfig(), - enabled: canViewDeploymentSettingsPage, + enabled: permissions.viewDeploymentValues, }); - // Need to make sure that we redirect basic users. If the enabled value for - // deploymentConfigQuery is false, the query object will be stuck in a - // loading state forever - if (!canViewDeploymentSettingsPage) { - return ; - } - - if (deploymentConfigQuery.isLoading) { + // Have to make check more specific, because if the query is disabled, it + // will be forever stuck in the loading state. The loading state is only + // relevant to owners right now + if (permissions.viewDeploymentValues && deploymentConfigQuery.isLoading) { return ; } return ( - +
- {deploymentConfigQuery.isError ? ( + {deploymentConfigQuery.isError && ( - ) : ( - - }> - - - )} + + + }> + + +
diff --git a/site/src/modules/management/SidebarView.stories.tsx b/site/src/modules/management/SidebarView.stories.tsx index 2ddcf7750bc8d..62fdce3ed728a 100644 --- a/site/src/modules/management/SidebarView.stories.tsx +++ b/site/src/modules/management/SidebarView.stories.tsx @@ -1,5 +1,6 @@ import type { Meta, StoryObj } from "@storybook/react"; import { + MockNoPermissions, MockOrganization, MockOrganization2, MockPermissions, @@ -95,9 +96,7 @@ export const NoDeploymentValues: Story = { }; export const NoPermissions: Story = { - args: { - permissions: {}, - }, + args: { permissions: MockNoPermissions }, }; export const NoSelected: Story = { diff --git a/site/src/modules/management/SidebarView.tsx b/site/src/modules/management/SidebarView.tsx index b4099a4dd7815..7439c82223ae7 100644 --- a/site/src/modules/management/SidebarView.tsx +++ b/site/src/modules/management/SidebarView.tsx @@ -2,19 +2,15 @@ import { cx } from "@emotion/css"; import type { Interpolation, Theme } from "@emotion/react"; import AddIcon from "@mui/icons-material/Add"; import SettingsIcon from "@mui/icons-material/Settings"; -import type { - AuthorizationResponse, - Experiments, - Organization, -} from "api/typesGenerated"; +import type { AuthorizationResponse, Organization } from "api/typesGenerated"; import { FeatureStageBadge } from "components/FeatureStageBadge/FeatureStageBadge"; import { Loader } from "components/Loader/Loader"; import { Sidebar as BaseSidebar } from "components/Sidebar/Sidebar"; import { Stack } from "components/Stack/Stack"; import { UserAvatar } from "components/UserAvatar/UserAvatar"; +import type { Permissions } from "contexts/auth/permissions"; import { type ClassName, useClassName } from "hooks/useClassName"; import { useDashboard } from "modules/dashboard/useDashboard"; -import { linkToUsers } from "modules/navigation"; import type { FC, ReactNode } from "react"; import { Link, NavLink } from "react-router-dom"; @@ -30,7 +26,7 @@ interface SidebarProps { /** Organizations and their permissions or undefined if still fetching. */ organizations: OrganizationWithPermissions[] | undefined; /** Site-wide permissions. */ - permissions: AuthorizationResponse; + permissions: Permissions; } /** @@ -72,7 +68,7 @@ interface DeploymentSettingsNavigationProps { /** Whether a deployment setting page is being viewed. */ active: boolean; /** Site-wide permissions. */ - permissions: AuthorizationResponse; + permissions: Permissions; } /** @@ -130,10 +126,11 @@ const DeploymentSettingsNavigation: FC = ({ {permissions.viewDeploymentValues && ( Network )} - {/* All users can view workspace regions. */} - - Workspace Proxies - + {permissions.readWorkspaceProxies && ( + + Workspace Proxies + + )} {permissions.viewDeploymentValues && ( Security )} @@ -145,12 +142,14 @@ const DeploymentSettingsNavigation: FC = ({ {permissions.viewAllUsers && ( Users )} - - - Notifications - - - + {permissions.viewNotificationTemplate && ( + + + Notifications + + + + )} )} @@ -167,7 +166,7 @@ interface OrganizationsSettingsNavigationProps { /** Organizations and their permissions or undefined if still fetching. */ organizations: OrganizationWithPermissions[] | undefined; /** Site-wide permissions. */ - permissions: AuthorizationResponse; + permissions: Permissions; } /** diff --git a/site/src/testHelpers/entities.ts b/site/src/testHelpers/entities.ts index 0db6e80d435d6..1593790e9792d 100644 --- a/site/src/testHelpers/entities.ts +++ b/site/src/testHelpers/entities.ts @@ -2766,12 +2766,37 @@ export const MockPermissions: Permissions = { viewUpdateCheck: true, viewDeploymentStats: true, viewExternalAuthConfig: true, + readWorkspaceProxies: true, editWorkspaceProxies: true, createOrganization: true, editAnyOrganization: true, viewAnyGroup: true, createGroup: true, viewAllLicenses: true, + viewNotificationTemplate: true, +}; + +export const MockNoPermissions: Permissions = { + createTemplates: false, + createUser: false, + deleteTemplates: false, + updateTemplates: false, + viewAllUsers: false, + updateUsers: false, + viewAnyAuditLog: false, + viewDeploymentValues: false, + editDeploymentValues: false, + viewUpdateCheck: false, + viewDeploymentStats: false, + viewExternalAuthConfig: false, + readWorkspaceProxies: false, + editWorkspaceProxies: false, + createOrganization: false, + editAnyOrganization: false, + viewAnyGroup: false, + createGroup: false, + viewAllLicenses: false, + viewNotificationTemplate: false, }; export const MockDeploymentConfig: DeploymentConfig = { From 36e03760b1735f44c298ef84eca8835d639821bd Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Thu, 24 Oct 2024 22:01:28 +0000 Subject: [PATCH 05/30] fix: update type misalignment --- site/jest.setup.ts | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/site/jest.setup.ts b/site/jest.setup.ts index 7a06ebba2592f..a8aa13490fd51 100644 --- a/site/jest.setup.ts +++ b/site/jest.setup.ts @@ -1,7 +1,7 @@ import "@testing-library/jest-dom"; import "jest-location-mock"; import { cleanup } from "@testing-library/react"; -import crypto from "crypto"; +import crypto from "node:crypto"; import { useMemo } from "react"; import type { Region } from "api/typesGenerated"; import type { ProxyLatencyReport } from "contexts/useProxyLatency"; @@ -48,9 +48,7 @@ global.ResizeObserver = require("resize-observer-polyfill"); // Polyfill the getRandomValues that is used on utils/random.ts Object.defineProperty(global.self, "crypto", { value: { - getRandomValues: function (buffer: Buffer) { - return crypto.randomFillSync(buffer); - }, + getRandomValues: (b: NodeJS.ArrayBufferView) => crypto.randomFillSync(b), }, }); @@ -72,5 +70,5 @@ afterEach(() => { // Clean up after the tests are finished. afterAll(() => server.close()); -// This is needed because we are compiling under `--isolatedModules` +// biome-ignore lint/complexity/noUselessEmptyExport: This is needed because we are compiling under `--isolatedModules` export {}; From 032a75c77d52c1cfbd75f4e47a9e96e53b7d5d28 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Thu, 24 Oct 2024 22:02:57 +0000 Subject: [PATCH 06/30] chore: format and tidy up --- site/src/modules/dashboard/DashboardProvider.tsx | 2 +- .../CustomRolesPage/CreateEditRolePage.tsx | 2 +- .../ManagementSettingsPage/CustomRolesPage/CustomRolesPage.tsx | 2 +- site/src/pages/ManagementSettingsPage/GroupsPage/GroupsPage.tsx | 2 +- .../pages/ManagementSettingsPage/IdpSyncPage/IdpSyncPage.tsx | 2 +- .../pages/ManagementSettingsPage/OrganizationMembersPage.tsx | 2 +- .../pages/ManagementSettingsPage/OrganizationSettingsPage.tsx | 2 +- 7 files changed, 7 insertions(+), 7 deletions(-) diff --git a/site/src/modules/dashboard/DashboardProvider.tsx b/site/src/modules/dashboard/DashboardProvider.tsx index 52dc8dba346b2..8dee68a2c53da 100644 --- a/site/src/modules/dashboard/DashboardProvider.tsx +++ b/site/src/modules/dashboard/DashboardProvider.tsx @@ -13,8 +13,8 @@ import { Loader } from "components/Loader/Loader"; import { useEmbeddedMetadata } from "hooks/useEmbeddedMetadata"; import { type FC, type PropsWithChildren, createContext } from "react"; import { useQuery } from "react-query"; -import { selectFeatureVisibility } from "./entitlements"; import { useParams } from "react-router-dom"; +import { selectFeatureVisibility } from "./entitlements"; export interface DashboardValue { entitlements: Entitlements; diff --git a/site/src/pages/ManagementSettingsPage/CustomRolesPage/CreateEditRolePage.tsx b/site/src/pages/ManagementSettingsPage/CustomRolesPage/CreateEditRolePage.tsx index c46dc4c2b1c9b..6701a52e68f63 100644 --- a/site/src/pages/ManagementSettingsPage/CustomRolesPage/CreateEditRolePage.tsx +++ b/site/src/pages/ManagementSettingsPage/CustomRolesPage/CreateEditRolePage.tsx @@ -8,13 +8,13 @@ import { import type { CustomRoleRequest } from "api/typesGenerated"; import { displayError } from "components/GlobalSnackbar/utils"; import { Loader } from "components/Loader/Loader"; +import { useDashboard } from "modules/dashboard/useDashboard"; import type { FC } from "react"; import { Helmet } from "react-helmet-async"; import { useMutation, useQuery, useQueryClient } from "react-query"; import { useNavigate, useParams } from "react-router-dom"; import { pageTitle } from "utils/page"; import CreateEditRolePageView from "./CreateEditRolePageView"; -import { useDashboard } from "modules/dashboard/useDashboard"; export const CreateEditRolePage: FC = () => { const queryClient = useQueryClient(); diff --git a/site/src/pages/ManagementSettingsPage/CustomRolesPage/CustomRolesPage.tsx b/site/src/pages/ManagementSettingsPage/CustomRolesPage/CustomRolesPage.tsx index 1d78e9bf79e08..d6b225629051d 100644 --- a/site/src/pages/ManagementSettingsPage/CustomRolesPage/CustomRolesPage.tsx +++ b/site/src/pages/ManagementSettingsPage/CustomRolesPage/CustomRolesPage.tsx @@ -7,6 +7,7 @@ import { displayError, displaySuccess } from "components/GlobalSnackbar/utils"; import { Loader } from "components/Loader/Loader"; import { SettingsHeader } from "components/SettingsHeader/SettingsHeader"; import { Stack } from "components/Stack/Stack"; +import { useDashboard } from "modules/dashboard/useDashboard"; import { useFeatureVisibility } from "modules/dashboard/useFeatureVisibility"; import { type FC, useEffect, useState } from "react"; import { Helmet } from "react-helmet-async"; @@ -14,7 +15,6 @@ import { useMutation, useQuery, useQueryClient } from "react-query"; import { useParams } from "react-router-dom"; import { pageTitle } from "utils/page"; import CustomRolesPageView from "./CustomRolesPageView"; -import { useDashboard } from "modules/dashboard/useDashboard"; export const CustomRolesPage: FC = () => { const queryClient = useQueryClient(); diff --git a/site/src/pages/ManagementSettingsPage/GroupsPage/GroupsPage.tsx b/site/src/pages/ManagementSettingsPage/GroupsPage/GroupsPage.tsx index 9d2f20f9d3b07..1524ac944cce7 100644 --- a/site/src/pages/ManagementSettingsPage/GroupsPage/GroupsPage.tsx +++ b/site/src/pages/ManagementSettingsPage/GroupsPage/GroupsPage.tsx @@ -9,6 +9,7 @@ import { displayError } from "components/GlobalSnackbar/utils"; import { Loader } from "components/Loader/Loader"; import { SettingsHeader } from "components/SettingsHeader/SettingsHeader"; import { Stack } from "components/Stack/Stack"; +import { useDashboard } from "modules/dashboard/useDashboard"; import { useFeatureVisibility } from "modules/dashboard/useFeatureVisibility"; import { type FC, useEffect } from "react"; import { Helmet } from "react-helmet-async"; @@ -16,7 +17,6 @@ import { useQuery } from "react-query"; import { Navigate, Link as RouterLink, useParams } from "react-router-dom"; import { pageTitle } from "utils/page"; import GroupsPageView from "./GroupsPageView"; -import { useDashboard } from "modules/dashboard/useDashboard"; export const GroupsPage: FC = () => { const feats = useFeatureVisibility(); diff --git a/site/src/pages/ManagementSettingsPage/IdpSyncPage/IdpSyncPage.tsx b/site/src/pages/ManagementSettingsPage/IdpSyncPage/IdpSyncPage.tsx index 9c3b908bd7cb9..cccc4eb9d386a 100644 --- a/site/src/pages/ManagementSettingsPage/IdpSyncPage/IdpSyncPage.tsx +++ b/site/src/pages/ManagementSettingsPage/IdpSyncPage/IdpSyncPage.tsx @@ -10,6 +10,7 @@ import { EmptyState } from "components/EmptyState/EmptyState"; import { Paywall } from "components/Paywall/Paywall"; import { SettingsHeader } from "components/SettingsHeader/SettingsHeader"; import { Stack } from "components/Stack/Stack"; +import { useDashboard } from "modules/dashboard/useDashboard"; import { useFeatureVisibility } from "modules/dashboard/useFeatureVisibility"; import type { FC } from "react"; import { Helmet } from "react-helmet-async"; @@ -19,7 +20,6 @@ import { docs } from "utils/docs"; import { pageTitle } from "utils/page"; import { IdpSyncHelpTooltip } from "./IdpSyncHelpTooltip"; import IdpSyncPageView from "./IdpSyncPageView"; -import { useDashboard } from "modules/dashboard/useDashboard"; export const IdpSyncPage: FC = () => { const { organization: organizationName } = useParams() as { diff --git a/site/src/pages/ManagementSettingsPage/OrganizationMembersPage.tsx b/site/src/pages/ManagementSettingsPage/OrganizationMembersPage.tsx index 22e80901cc9bf..c0de1ef1bc16d 100644 --- a/site/src/pages/ManagementSettingsPage/OrganizationMembersPage.tsx +++ b/site/src/pages/ManagementSettingsPage/OrganizationMembersPage.tsx @@ -15,11 +15,11 @@ import { displayError, displaySuccess } from "components/GlobalSnackbar/utils"; import { Loader } from "components/Loader/Loader"; import { Stack } from "components/Stack/Stack"; import { useAuthenticated } from "contexts/auth/RequireAuth"; +import { useDashboard } from "modules/dashboard/useDashboard"; import { type FC, useState } from "react"; import { useMutation, useQuery, useQueryClient } from "react-query"; import { useParams } from "react-router-dom"; import { OrganizationMembersPageView } from "./OrganizationMembersPageView"; -import { useDashboard } from "modules/dashboard/useDashboard"; const OrganizationMembersPage: FC = () => { const queryClient = useQueryClient(); diff --git a/site/src/pages/ManagementSettingsPage/OrganizationSettingsPage.tsx b/site/src/pages/ManagementSettingsPage/OrganizationSettingsPage.tsx index 2a44dad3a3f19..00fe8b564418f 100644 --- a/site/src/pages/ManagementSettingsPage/OrganizationSettingsPage.tsx +++ b/site/src/pages/ManagementSettingsPage/OrganizationSettingsPage.tsx @@ -8,6 +8,7 @@ import { ErrorAlert } from "components/Alert/ErrorAlert"; import { EmptyState } from "components/EmptyState/EmptyState"; import { displaySuccess } from "components/GlobalSnackbar/utils"; import { Loader } from "components/Loader/Loader"; +import { useDashboard } from "modules/dashboard/useDashboard"; import { useFeatureVisibility } from "modules/dashboard/useFeatureVisibility"; import { canEditOrganization } from "modules/management/ManagementSettingsLayout"; import type { FC } from "react"; @@ -15,7 +16,6 @@ import { useMutation, useQuery, useQueryClient } from "react-query"; import { Navigate, useNavigate, useParams } from "react-router-dom"; import { OrganizationSettingsPageView } from "./OrganizationSettingsPageView"; import { OrganizationSummaryPageView } from "./OrganizationSummaryPageView"; -import { useDashboard } from "modules/dashboard/useDashboard"; const OrganizationSettingsPage: FC = () => { const { organizations, activeOrganization } = useDashboard(); From eea8bbde36b3c2794b24fc63d8bcb2c67e5fa604 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Thu, 24 Oct 2024 22:19:16 +0000 Subject: [PATCH 07/30] fix: make sure you can't navigate directly to bare /deployment page --- site/src/router.tsx | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/site/src/router.tsx b/site/src/router.tsx index 2531c823b9f48..3747ffa610801 100644 --- a/site/src/router.tsx +++ b/site/src/router.tsx @@ -427,6 +427,15 @@ export const router = createBrowserRouter( }> + {/* + None of the UI elements link directly to the base + /deployment route, but if you navigate there directly, + you would just see a mostly empty screen. Redirect to + the general page for better UX. */} + } + /> } /> } /> } /> From e6d4201c07223fc4662bd63f538808341cc90cc6 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Thu, 24 Oct 2024 23:07:49 +0000 Subject: [PATCH 08/30] fix: add more granularity to redirect logic --- .vscode/settings.json | 2 + site/src/contexts/auth/RequirePermission.tsx | 10 ++- .../management/ManagementSettingsLayout.tsx | 74 ++++++++++++++++--- 3 files changed, 73 insertions(+), 13 deletions(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index 2476e330cd306..cc4986706f7f8 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -174,7 +174,9 @@ "typesafe", "unauthenticate", "unconvert", + "unpermitted", "untar", + "userauth", "userspace", "VMID", "walkthrough", diff --git a/site/src/contexts/auth/RequirePermission.tsx b/site/src/contexts/auth/RequirePermission.tsx index 50dbd0232ab88..f2beb67dfb9ae 100644 --- a/site/src/contexts/auth/RequirePermission.tsx +++ b/site/src/contexts/auth/RequirePermission.tsx @@ -3,7 +3,8 @@ import { Navigate } from "react-router-dom"; export interface RequirePermissionProps { children?: ReactNode; - isFeatureVisible: boolean; + permitted: boolean; + unpermittedRedirect?: `/${string}`; } /** @@ -11,10 +12,11 @@ export interface RequirePermissionProps { */ export const RequirePermission: FC = ({ children, - isFeatureVisible, + permitted, + unpermittedRedirect = "/workspaces", }) => { - if (!isFeatureVisible) { - return ; + if (!permitted) { + return ; } return <>{children}; diff --git a/site/src/modules/management/ManagementSettingsLayout.tsx b/site/src/modules/management/ManagementSettingsLayout.tsx index 920b075f94a5f..2cb9f3d1c458b 100644 --- a/site/src/modules/management/ManagementSettingsLayout.tsx +++ b/site/src/modules/management/ManagementSettingsLayout.tsx @@ -9,8 +9,9 @@ import { useAuthenticated } from "contexts/auth/RequireAuth"; import { RequirePermission } from "contexts/auth/RequirePermission"; import { type FC, Suspense, createContext, useContext } from "react"; import { useQuery } from "react-query"; -import { Outlet } from "react-router-dom"; +import { Outlet, useLocation } from "react-router-dom"; import { Sidebar } from "./Sidebar"; +import type { Permissions } from "contexts/auth/permissions"; type ManagementSettingsValue = Readonly<{ deploymentValues: DeploymentConfig | undefined; @@ -45,6 +46,54 @@ export const canEditOrganization = ( ); }; +const isManagementRoutePermitted = ( + href: string, + permissions: Permissions, +): boolean => { + console.log(href); + + // Switch logic should mirror the conditions used to display the sidebar + // tabs from SidebarView.tsx + switch (href) { + case "/general": { + return permissions.viewDeploymentValues; + } + case "/licenses": { + return permissions.viewAllLicenses; + } + case "/appearance": { + return permissions.editDeploymentValues; + } + case "/userauth": { + return permissions.viewDeploymentValues; + } + case "/external-auth": { + return permissions.viewDeploymentValues; + } + case "/network": { + return permissions.viewDeploymentValues; + } + case "/workspace-proxies": { + return permissions.readWorkspaceProxies; + } + case "/security": { + return permissions.viewDeploymentValues; + } + case "/observability": { + return permissions.viewDeploymentValues; + } + case "/users": { + return permissions.viewAllUsers; + } + case "/notifications": { + return permissions.viewNotificationTemplate; + } + default: { + return false; + } + } +}; + /** * A multi-org capable settings page layout. * @@ -52,6 +101,7 @@ export const canEditOrganization = ( * See DeploySettingsLayoutInner instead. */ export const ManagementSettingsLayout: FC = () => { + const location = useLocation(); const { permissions } = useAuthenticated(); const deploymentConfigQuery = useQuery({ ...deploymentConfig(), @@ -65,16 +115,22 @@ export const ManagementSettingsLayout: FC = () => { return ; } + const href = location.pathname.replace(/^\/?deployment/, ""); + const canViewSomePageContent = + permissions.viewDeploymentValues || + permissions.viewAllUsers || + permissions.editAnyOrganization || + permissions.viewAnyAuditLog; + return ( From 14fc68e658f22e0c886781a6d75c498ef6c61929 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Thu, 24 Oct 2024 23:17:02 +0000 Subject: [PATCH 09/30] fix: prevent infinite redirect --- .../management/ManagementSettingsLayout.tsx | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/site/src/modules/management/ManagementSettingsLayout.tsx b/site/src/modules/management/ManagementSettingsLayout.tsx index 2cb9f3d1c458b..6d84a71cbbc41 100644 --- a/site/src/modules/management/ManagementSettingsLayout.tsx +++ b/site/src/modules/management/ManagementSettingsLayout.tsx @@ -116,7 +116,7 @@ export const ManagementSettingsLayout: FC = () => { } const href = location.pathname.replace(/^\/?deployment/, ""); - const canViewSomePageContent = + const canViewAtLeastOneTab = permissions.viewDeploymentValues || permissions.viewAllUsers || permissions.editAnyOrganization || @@ -125,10 +125,22 @@ export const ManagementSettingsLayout: FC = () => { return ( Date: Thu, 24 Oct 2024 23:47:33 +0000 Subject: [PATCH 10/30] fix: update redirects for /organizations routes --- .../management/ManagementSettingsLayout.tsx | 45 ++++++++++--------- site/src/router.tsx | 26 +++++++---- 2 files changed, 42 insertions(+), 29 deletions(-) diff --git a/site/src/modules/management/ManagementSettingsLayout.tsx b/site/src/modules/management/ManagementSettingsLayout.tsx index 6d84a71cbbc41..0ab70ecf634db 100644 --- a/site/src/modules/management/ManagementSettingsLayout.tsx +++ b/site/src/modules/management/ManagementSettingsLayout.tsx @@ -12,6 +12,7 @@ import { useQuery } from "react-query"; import { Outlet, useLocation } from "react-router-dom"; import { Sidebar } from "./Sidebar"; import type { Permissions } from "contexts/auth/permissions"; +import { useDashboard } from "modules/dashboard/useDashboard"; type ManagementSettingsValue = Readonly<{ deploymentValues: DeploymentConfig | undefined; @@ -47,14 +48,25 @@ export const canEditOrganization = ( }; const isManagementRoutePermitted = ( - href: string, + locationPath: string, permissions: Permissions, + showOrganizations: boolean, ): boolean => { - console.log(href); + if (locationPath.startsWith("/organizations")) { + return showOrganizations; + } + + if (!locationPath.startsWith("/deployment")) { + return false; + } - // Switch logic should mirror the conditions used to display the sidebar - // tabs from SidebarView.tsx + // Switch logic for deployment routes should mirror the conditions used to + // display the sidebar tabs from SidebarView.tsx + const href = locationPath.replace(/^\/deployment/, ""); switch (href) { + case "/": { + return true; + } case "/general": { return permissions.viewDeploymentValues; } @@ -102,6 +114,7 @@ const isManagementRoutePermitted = ( */ export const ManagementSettingsLayout: FC = () => { const location = useLocation(); + const { showOrganizations } = useDashboard(); const { permissions } = useAuthenticated(); const deploymentConfigQuery = useQuery({ ...deploymentConfig(), @@ -115,7 +128,6 @@ export const ManagementSettingsLayout: FC = () => { return ; } - const href = location.pathname.replace(/^\/?deployment/, ""); const canViewAtLeastOneTab = permissions.viewDeploymentValues || permissions.viewAllUsers || @@ -125,23 +137,16 @@ export const ManagementSettingsLayout: FC = () => { return ( diff --git a/site/src/router.tsx b/site/src/router.tsx index 3747ffa610801..68c35764e5eb5 100644 --- a/site/src/router.tsx +++ b/site/src/router.tsx @@ -426,16 +426,24 @@ export const router = createBrowserRouter( + {/** + * @todo 2024-10-24 - MES - The current deployment page is + * almost empty, which isn't great for two reasons: + * + * 1. Even though none of our links lead to the page, the + * user is still able to navigate straight there by + * accident. + * 2. If the user has access to some deployment pages, but + * tries accessing one that they shouldn't be able to + * access, we have to reroute them somewhere. The base + * deployment route is the only safe option, because not + * even the general page is accessible by everyone. + * + * Should update the base /deployment page so that there's + * something there, but that will require coordination with + * the design team. + */} }> - {/* - None of the UI elements link directly to the base - /deployment route, but if you navigate there directly, - you would just see a mostly empty screen. Redirect to - the general page for better UX. */} - } - /> } /> } /> } /> From 0a36cd113891cdacd7293f70335ab3982156be0d Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Thu, 24 Oct 2024 23:52:30 +0000 Subject: [PATCH 11/30] fix: format files --- site/src/modules/management/ManagementSettingsLayout.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/site/src/modules/management/ManagementSettingsLayout.tsx b/site/src/modules/management/ManagementSettingsLayout.tsx index 0ab70ecf634db..d85dcba72ef2b 100644 --- a/site/src/modules/management/ManagementSettingsLayout.tsx +++ b/site/src/modules/management/ManagementSettingsLayout.tsx @@ -7,12 +7,12 @@ import { Margins } from "components/Margins/Margins"; import { Stack } from "components/Stack/Stack"; import { useAuthenticated } from "contexts/auth/RequireAuth"; import { RequirePermission } from "contexts/auth/RequirePermission"; +import type { Permissions } from "contexts/auth/permissions"; +import { useDashboard } from "modules/dashboard/useDashboard"; import { type FC, Suspense, createContext, useContext } from "react"; import { useQuery } from "react-query"; import { Outlet, useLocation } from "react-router-dom"; import { Sidebar } from "./Sidebar"; -import type { Permissions } from "contexts/auth/permissions"; -import { useDashboard } from "modules/dashboard/useDashboard"; type ManagementSettingsValue = Readonly<{ deploymentValues: DeploymentConfig | undefined; From e6d772b31888d4928a7d6d8b3089c8d4339d0a55 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Fri, 25 Oct 2024 00:06:14 +0000 Subject: [PATCH 12/30] fix: tighten up types for permissions --- site/src/contexts/auth/permissions.tsx | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/site/src/contexts/auth/permissions.tsx b/site/src/contexts/auth/permissions.tsx index b8996a623ea02..a4e39fb1bee01 100644 --- a/site/src/contexts/auth/permissions.tsx +++ b/site/src/contexts/auth/permissions.tsx @@ -23,6 +23,8 @@ export const checks = { viewNotificationTemplate: "viewNotificationTemplate", } as const satisfies Record; +type PermissionType = keyof typeof checks; + export const permissionsToCheck = { [checks.viewAllUsers]: { object: { @@ -147,6 +149,6 @@ export const permissionsToCheck = { }, action: "read", }, -} as const satisfies Record; +} as const satisfies Record; -export type Permissions = Record; +export type Permissions = Record; From 2068a4c4d257cdf17105d9f8157921c27a069cf9 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Fri, 25 Oct 2024 14:28:24 +0000 Subject: [PATCH 13/30] fix: update route checks to account for subroutes --- .../management/ManagementSettingsLayout.tsx | 78 +++++++++---------- 1 file changed, 38 insertions(+), 40 deletions(-) diff --git a/site/src/modules/management/ManagementSettingsLayout.tsx b/site/src/modules/management/ManagementSettingsLayout.tsx index d85dcba72ef2b..09e87043f26ba 100644 --- a/site/src/modules/management/ManagementSettingsLayout.tsx +++ b/site/src/modules/management/ManagementSettingsLayout.tsx @@ -63,47 +63,45 @@ const isManagementRoutePermitted = ( // Switch logic for deployment routes should mirror the conditions used to // display the sidebar tabs from SidebarView.tsx const href = locationPath.replace(/^\/deployment/, ""); - switch (href) { - case "/": { - return true; - } - case "/general": { - return permissions.viewDeploymentValues; - } - case "/licenses": { - return permissions.viewAllLicenses; - } - case "/appearance": { - return permissions.editDeploymentValues; - } - case "/userauth": { - return permissions.viewDeploymentValues; - } - case "/external-auth": { - return permissions.viewDeploymentValues; - } - case "/network": { - return permissions.viewDeploymentValues; - } - case "/workspace-proxies": { - return permissions.readWorkspaceProxies; - } - case "/security": { - return permissions.viewDeploymentValues; - } - case "/observability": { - return permissions.viewDeploymentValues; - } - case "/users": { - return permissions.viewAllUsers; - } - case "/notifications": { - return permissions.viewNotificationTemplate; - } - default: { - return false; - } + + if (href === "/" || href === "") { + return true; + } + if (href.startsWith("/general")) { + return permissions.viewDeploymentValues; + } + if (href.startsWith("/licenses")) { + return permissions.viewAllLicenses; + } + if (href.startsWith("/appearance")) { + return permissions.editDeploymentValues; + } + if (href.startsWith("/userauth")) { + return permissions.viewDeploymentValues; + } + if (href.startsWith("/external-auth")) { + return permissions.viewDeploymentValues; + } + if (href.startsWith("/network")) { + return permissions.viewDeploymentValues; } + if (href.startsWith("/workspace-proxies")) { + return permissions.readWorkspaceProxies; + } + if (href.startsWith("/security")) { + return permissions.viewDeploymentValues; + } + if (href.startsWith("/observability")) { + return permissions.viewDeploymentValues; + } + if (href.startsWith("/users")) { + return permissions.viewAllUsers; + } + if (href.startsWith("/notifications")) { + return permissions.viewNotificationTemplate; + } + + return false; }; /** From 2fe3d539df6d8205f55de88008d49040e1b2efb0 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Fri, 25 Oct 2024 14:31:48 +0000 Subject: [PATCH 14/30] fix: add e2e check to ensure that redirect hasn't happened --- site/e2e/global.setup.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/site/e2e/global.setup.ts b/site/e2e/global.setup.ts index 6eafd2886de37..f39a2d475804e 100644 --- a/site/e2e/global.setup.ts +++ b/site/e2e/global.setup.ts @@ -35,6 +35,7 @@ test("setup deployment", async ({ page }) => { expect(constants.license.split(".").length).toBe(3); // otherwise it's invalid await page.goto("/deployment/licenses", { waitUntil: "domcontentloaded" }); + await expect(page).toHaveTitle("License Settings - Coder"); await page.getByText("Add a license").click(); await page.getByRole("textbox").fill(constants.license); From 3c41de0c85427b78c021b0060d23666f5be25198 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Fri, 25 Oct 2024 14:56:16 +0000 Subject: [PATCH 15/30] fix: update stories to account for new dashboard logic --- .../OrganizationSettingsPage.stories.tsx | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/site/src/pages/ManagementSettingsPage/OrganizationSettingsPage.stories.tsx b/site/src/pages/ManagementSettingsPage/OrganizationSettingsPage.stories.tsx index 9c85f89a62b55..6b2c5348c7bd7 100644 --- a/site/src/pages/ManagementSettingsPage/OrganizationSettingsPage.stories.tsx +++ b/site/src/pages/ManagementSettingsPage/OrganizationSettingsPage.stories.tsx @@ -33,10 +33,15 @@ const meta: Meta = { export default meta; type Story = StoryObj; -export const NoRedirectableOrganizations: Story = {}; +export const NoRedirectableOrganizations: Story = { + parameters: { + activeOrganization: undefined, + }, +}; export const OrganizationDoesNotExist: Story = { parameters: { + activeOrganization: undefined, reactRouter: reactRouterParameters({ location: { pathParams: { organization: "does-not-exist" } }, routing: { path: "/organizations/:organization" }, From 162642e3c4b0bc73bdb59031d63d8a3814856d68 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Fri, 25 Oct 2024 14:59:06 +0000 Subject: [PATCH 16/30] fix: update out-of-date comment --- site/src/modules/management/ManagementSettingsLayout.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/site/src/modules/management/ManagementSettingsLayout.tsx b/site/src/modules/management/ManagementSettingsLayout.tsx index 09e87043f26ba..abfc197b0b639 100644 --- a/site/src/modules/management/ManagementSettingsLayout.tsx +++ b/site/src/modules/management/ManagementSettingsLayout.tsx @@ -60,8 +60,8 @@ const isManagementRoutePermitted = ( return false; } - // Switch logic for deployment routes should mirror the conditions used to - // display the sidebar tabs from SidebarView.tsx + // Logic for deployment routes should mirror the conditions used to display + // the sidebar tabs from SidebarView.tsx const href = locationPath.replace(/^\/deployment/, ""); if (href === "/" || href === "") { From 64c6f2940705d97220e2582a466ac0855c22d09c Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Fri, 25 Oct 2024 15:24:34 +0000 Subject: [PATCH 17/30] fix: update default value logic for storybook setup --- site/src/testHelpers/storybook.tsx | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/site/src/testHelpers/storybook.tsx b/site/src/testHelpers/storybook.tsx index cae8e7714ae62..2e9b91deac626 100644 --- a/site/src/testHelpers/storybook.tsx +++ b/site/src/testHelpers/storybook.tsx @@ -25,10 +25,17 @@ export const withDashboardProvider = ( features = [], experiments = [], showOrganizations = false, - organizations = [MockDefaultOrganization], - activeOrganization = organizations[0] ?? MockDefaultOrganization, } = parameters; + // Only set a default value for activeOrganizations if the original + // organizations array wasn't specified. In some cases, we want to have a + // list of organizations, but not have one be active + let { organizations, activeOrganization } = parameters; + if (organizations === undefined) { + organizations = [MockDefaultOrganization]; + activeOrganization = MockDefaultOrganization; + } + const entitlements: Entitlements = { ...MockEntitlements, has_license: features.length > 0, From e12a4aacba577a7eb3a3385b60f05e4127515798 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Fri, 25 Oct 2024 15:31:11 +0000 Subject: [PATCH 18/30] fix: update individual stories --- .../OrganizationSettingsPage.stories.tsx | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/site/src/pages/ManagementSettingsPage/OrganizationSettingsPage.stories.tsx b/site/src/pages/ManagementSettingsPage/OrganizationSettingsPage.stories.tsx index 6b2c5348c7bd7..6c6c302256440 100644 --- a/site/src/pages/ManagementSettingsPage/OrganizationSettingsPage.stories.tsx +++ b/site/src/pages/ManagementSettingsPage/OrganizationSettingsPage.stories.tsx @@ -1,6 +1,10 @@ import type { Meta, StoryObj } from "@storybook/react"; import { reactRouterParameters } from "storybook-addon-remix-react-router"; -import { MockDefaultOrganization, MockUser } from "testHelpers/entities"; +import { + MockDefaultOrganization, + MockOrganization, + MockUser, +} from "testHelpers/entities"; import { withAuthProvider, withDashboardProvider, @@ -35,13 +39,18 @@ type Story = StoryObj; export const NoRedirectableOrganizations: Story = { parameters: { + organizations: [], activeOrganization: undefined, }, }; export const OrganizationDoesNotExist: Story = { parameters: { - activeOrganization: undefined, + organizations: [MockDefaultOrganization], + activeOrganization: { + ...MockOrganization, + name: "does-not-exist", + }, reactRouter: reactRouterParameters({ location: { pathParams: { organization: "does-not-exist" } }, routing: { path: "/organizations/:organization" }, From b9b33c0d37694bb94e60da6a5997f9062931f934 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Fri, 25 Oct 2024 15:57:01 +0000 Subject: [PATCH 19/30] fix: finish storybook injection changes --- .../OrganizationSettingsPage.stories.tsx | 8 +-- .../OrganizationSettingsPage.tsx | 59 +++++++++---------- 2 files changed, 32 insertions(+), 35 deletions(-) diff --git a/site/src/pages/ManagementSettingsPage/OrganizationSettingsPage.stories.tsx b/site/src/pages/ManagementSettingsPage/OrganizationSettingsPage.stories.tsx index 6c6c302256440..acdb088a64c04 100644 --- a/site/src/pages/ManagementSettingsPage/OrganizationSettingsPage.stories.tsx +++ b/site/src/pages/ManagementSettingsPage/OrganizationSettingsPage.stories.tsx @@ -3,6 +3,7 @@ import { reactRouterParameters } from "storybook-addon-remix-react-router"; import { MockDefaultOrganization, MockOrganization, + MockOrganization2, MockUser, } from "testHelpers/entities"; import { @@ -39,7 +40,7 @@ type Story = StoryObj; export const NoRedirectableOrganizations: Story = { parameters: { - organizations: [], + organizations: [MockDefaultOrganization], activeOrganization: undefined, }, }; @@ -47,10 +48,7 @@ export const NoRedirectableOrganizations: Story = { export const OrganizationDoesNotExist: Story = { parameters: { organizations: [MockDefaultOrganization], - activeOrganization: { - ...MockOrganization, - name: "does-not-exist", - }, + activeOrganization: MockOrganization2, reactRouter: reactRouterParameters({ location: { pathParams: { organization: "does-not-exist" } }, routing: { path: "/organizations/:organization" }, diff --git a/site/src/pages/ManagementSettingsPage/OrganizationSettingsPage.tsx b/site/src/pages/ManagementSettingsPage/OrganizationSettingsPage.tsx index 00fe8b564418f..6fa6fee2bc7a8 100644 --- a/site/src/pages/ManagementSettingsPage/OrganizationSettingsPage.tsx +++ b/site/src/pages/ManagementSettingsPage/OrganizationSettingsPage.tsx @@ -3,7 +3,6 @@ import { organizationsPermissions, updateOrganization, } from "api/queries/organizations"; -import type { Organization } from "api/typesGenerated"; import { ErrorAlert } from "components/Alert/ErrorAlert"; import { EmptyState } from "components/EmptyState/EmptyState"; import { displaySuccess } from "components/GlobalSnackbar/utils"; @@ -13,7 +12,7 @@ import { useFeatureVisibility } from "modules/dashboard/useFeatureVisibility"; import { canEditOrganization } from "modules/management/ManagementSettingsLayout"; import type { FC } from "react"; import { useMutation, useQuery, useQueryClient } from "react-query"; -import { Navigate, useNavigate, useParams } from "react-router-dom"; +import { Navigate, useNavigate } from "react-router-dom"; import { OrganizationSettingsPageView } from "./OrganizationSettingsPageView"; import { OrganizationSummaryPageView } from "./OrganizationSummaryPageView"; @@ -29,13 +28,8 @@ const OrganizationSettingsPage: FC = () => { const deleteOrganizationMutation = useMutation( deleteOrganization(queryClient), ); - - const organization = - organizations && activeOrganization - ? getOrganizationByName(organizations, activeOrganization.name) - : undefined; const permissionsQuery = useQuery( - organizationsPermissions(organizations?.map((o) => o.id)), + organizationsPermissions(organizations.map((o) => o.id)), ); if (permissionsQuery.isLoading) { @@ -47,25 +41,30 @@ const OrganizationSettingsPage: FC = () => { return ; } + if (!activeOrganization || !organizations.includes(activeOrganization)) { + return ; + } + // Redirect /organizations => /organizations/default-org, or if they cannot edit // the default org, then the first org they can edit, if any. - if (!activeOrganization?.name) { + if (!activeOrganization.name) { + // .find will stop at the first match found; make sure default + // organizations are placed first const editableOrg = [...organizations] .sort((a, b) => { - // Prefer default org (it may not be first). - // JavaScript will happily subtract booleans, but use numbers to keep - // the compiler happy. - return (b.is_default ? 1 : 0) - (a.is_default ? 1 : 0); + if (a.is_default && !b.is_default) { + return -1; + } + if (b.is_default && !a.is_default) { + return 1; + } + return 0; }) .find((org) => canEditOrganization(permissions[org.id])); + if (editableOrg) { return ; } - return ; - } - - if (!organization) { - return ; } // The user may not be able to edit this org but they can still see it because @@ -74,10 +73,17 @@ const OrganizationSettingsPage: FC = () => { // Similarly, if the feature is not entitled then the user will not be able to // edit the organization. if ( - !permissions[organization.id]?.editOrganization || + /** + * @todo 2024-10-25 - MES - Do not remove the ?. chaining, even though + * the compiler will let you do it + * + * We need to tighten up the compiler settings more so that we're forced + * to deal with the case where a key does not exist in a Record + */ + !permissions[activeOrganization.id]?.editOrganization || !feats.multiple_organizations ) { - return ; + return ; } const error = @@ -85,19 +91,19 @@ const OrganizationSettingsPage: FC = () => { return ( { const updatedOrganization = await updateOrganizationMutation.mutateAsync({ - organizationId: organization.id, + organizationId: activeOrganization.id, req: values, }); navigate(`/organizations/${updatedOrganization.name}`); displaySuccess("Organization settings updated."); }} onDeleteOrganization={() => { - deleteOrganizationMutation.mutate(organization.id); + deleteOrganizationMutation.mutate(activeOrganization.id); displaySuccess("Organization deleted."); navigate("/organizations"); }} @@ -106,10 +112,3 @@ const OrganizationSettingsPage: FC = () => { }; export default OrganizationSettingsPage; - -const getOrganizationByName = ( - organizations: readonly Organization[], - name: string, -) => { - return organizations.find((org) => org.name === name); -}; From f7df5ff9d0cd4e889e0a36297f058c3899511525 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Fri, 25 Oct 2024 16:16:59 +0000 Subject: [PATCH 20/30] fix: add back separate empty state to satisfy tests --- .../OrganizationSettingsPage.tsx | 32 ++++++++++--------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/site/src/pages/ManagementSettingsPage/OrganizationSettingsPage.tsx b/site/src/pages/ManagementSettingsPage/OrganizationSettingsPage.tsx index 6fa6fee2bc7a8..0ba1496673ffd 100644 --- a/site/src/pages/ManagementSettingsPage/OrganizationSettingsPage.tsx +++ b/site/src/pages/ManagementSettingsPage/OrganizationSettingsPage.tsx @@ -41,13 +41,9 @@ const OrganizationSettingsPage: FC = () => { return ; } - if (!activeOrganization || !organizations.includes(activeOrganization)) { - return ; - } - - // Redirect /organizations => /organizations/default-org, or if they cannot edit - // the default org, then the first org they can edit, if any. - if (!activeOrganization.name) { + // Redirect /organizations => /organizations/default-org, or if they cannot + // edit the default org, then the first org they can edit, if any. + if (!activeOrganization?.name) { // .find will stop at the first match found; make sure default // organizations are placed first const editableOrg = [...organizations] @@ -65,14 +61,20 @@ const OrganizationSettingsPage: FC = () => { if (editableOrg) { return ; } + + return ; + } + + if (!activeOrganization || !organizations.includes(activeOrganization)) { + return ; } - // The user may not be able to edit this org but they can still see it because - // they can edit members, etc. In this case they will be shown a read-only - // summary page instead of the settings form. - // Similarly, if the feature is not entitled then the user will not be able to - // edit the organization. - if ( + // The user may not be able to edit this org but they can still see it + // because they can edit members, etc. In this case they will be shown a + // read-only summary page instead of the settings form. + // Similarly, if the feature is not entitled then the user will not be able + // to edit the organization. + const userCannotEditOrg = /** * @todo 2024-10-25 - MES - Do not remove the ?. chaining, even though * the compiler will let you do it @@ -81,8 +83,8 @@ const OrganizationSettingsPage: FC = () => { * to deal with the case where a key does not exist in a Record */ !permissions[activeOrganization.id]?.editOrganization || - !feats.multiple_organizations - ) { + !feats.multiple_organizations; + if (userCannotEditOrg) { return ; } From 6925294dd0576e82f050d21e15e3628103fd1d41 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Fri, 25 Oct 2024 21:10:16 +0000 Subject: [PATCH 21/30] fix: add tests and update code to account for missing cases --- .../ManagementSettingsLayout.test.ts | 188 ++++++++++++++++++ .../management/ManagementSettingsLayout.tsx | 12 +- 2 files changed, 198 insertions(+), 2 deletions(-) create mode 100644 site/src/modules/management/ManagementSettingsLayout.test.ts diff --git a/site/src/modules/management/ManagementSettingsLayout.test.ts b/site/src/modules/management/ManagementSettingsLayout.test.ts new file mode 100644 index 0000000000000..ee0bde062eef6 --- /dev/null +++ b/site/src/modules/management/ManagementSettingsLayout.test.ts @@ -0,0 +1,188 @@ +import type { Permissions } from "contexts/auth/permissions"; +import { isManagementRoutePermitted } from "./ManagementSettingsLayout"; +import { MockNoPermissions, MockPermissions } from "testHelpers/entities"; +import { Permission } from "api/typesGenerated"; + +describe(isManagementRoutePermitted.name, () => { + describe("General behavior", () => { + it("Rejects malformed routes", () => { + const invalidRoutes: readonly string[] = [ + // It's expected that the hostname will be stripped off + "https://dev.coder.com/deployment/licenses/add", + + // Missing the leading / + "organizations", + ]; + + for (const r of invalidRoutes) { + } + + // Currently only checking whether route ends with / + const invalidRoute = "organizations"; + expect( + isManagementRoutePermitted(invalidRoute, MockPermissions, true), + ).toBe(false); + }); + }); + + describe("Organization routes", () => { + it("Delegates to showOrganizations value for any /organizations routes", () => { + expect( + isManagementRoutePermitted("/organizations", MockNoPermissions, true), + ).toBe(true); + + expect( + isManagementRoutePermitted( + "/organizations/nested/arbitrarily", + MockNoPermissions, + true, + ), + ).toBe(true); + + expect( + isManagementRoutePermitted( + "/organizations/sample-organization", + MockNoPermissions, + true, + ), + ).toBe(true); + + expect( + isManagementRoutePermitted("/organizations", MockPermissions, false), + ).toBe(false); + }); + }); + + describe("Deployment routes", () => { + it("Will never let the user through if they have no active permissions", () => { + let result = isManagementRoutePermitted( + "/deployment", + MockNoPermissions, + true, + ); + expect(result).toBe(false); + + result = isManagementRoutePermitted( + "/deployment/licenses", + MockNoPermissions, + true, + ); + expect(result).toBe(false); + + result = isManagementRoutePermitted( + "/deployment/appearance", + MockNoPermissions, + false, + ); + expect(result).toBe(false); + }); + + it("Will let users access base /deployment route if they have at least one permission", () => { + const mocks: readonly Permissions[] = [ + MockPermissions, + { + ...MockNoPermissions, + createGroup: true, + }, + { + ...MockNoPermissions, + createOrganization: true, + deleteTemplates: true, + }, + { + ...MockNoPermissions, + editDeploymentValues: true, + viewNotificationTemplate: true, + readWorkspaceProxies: true, + }, + ]; + + for (const m of mocks) { + expect(isManagementRoutePermitted("/deployment", m, true)).toBe(true); + expect(isManagementRoutePermitted("/deployment", m, false)).toBe(true); + } + }); + + it("Rejects unknown deployment routes", () => { + const sampleRoutes: readonly string[] = [ + "/deployment/definitely-not-right", + "/deployment/what-is-this", + ]; + + for (const r of sampleRoutes) { + const result = isManagementRoutePermitted(r, MockPermissions, true); + expect(result).toBe(false); + } + }); + + it("Supports deployment routes that are nested more than one level", () => { + const routes: readonly string[] = [ + "/deployment/licenses/add", + + // Including oauth2-provider routes, even though they're not + // currently exposed via the UI + "/deployment/oauth2-provider/apps", + "/deployment/oauth2-provider/apps/add", + ]; + + for (const r of routes) { + let result = isManagementRoutePermitted(r, MockPermissions, true); + expect(result).toBe(true); + + result = isManagementRoutePermitted(r, MockPermissions, false); + expect(result).toBe(true); + } + }); + + it("Granularly associates individual deployment routes with specific permissions", () => { + type PairTuple = readonly [ + route: `/${string}`, + permissionKey: keyof Permissions, + ]; + + const pairs: readonly PairTuple[] = [ + ["/general", "viewDeploymentValues"], + ["/licenses", "viewAllLicenses"], + ["/appearance", "editDeploymentValues"], + ["/userauth", "viewDeploymentValues"], + ["/external-auth", "viewDeploymentValues"], + ["/network", "viewDeploymentValues"], + ["/workspace-proxies", "readWorkspaceProxies"], + ["/security", "viewDeploymentValues"], + ["/observability", "viewDeploymentValues"], + ["/users", "viewAllUsers"], + ["/notifications", "viewNotificationTemplate"], + ["/oauth2-provider", "viewExternalAuthConfig"], + ]; + + for (const [route, permName] of pairs) { + // Using NoPermissions version as baseline to make sure that we + // don't get false positives from all permissions being set to + // true at the start + const mutablePermsCopy = { ...MockNoPermissions }; + const fullRoute = `/deployment${route}`; + + mutablePermsCopy[permName] = true; + let result = isManagementRoutePermitted( + fullRoute, + mutablePermsCopy, + true, + ); + expect(result).toBe(true); + + result = isManagementRoutePermitted( + `${fullRoute}/example-subpath`, + mutablePermsCopy, + true, + ); + expect(result).toBe(true); + + mutablePermsCopy[permName] = false; + result = isManagementRoutePermitted(fullRoute, mutablePermsCopy, true); + expect(result).toBe(false); + } + + expect.hasAssertions(); + }); + }); +}); diff --git a/site/src/modules/management/ManagementSettingsLayout.tsx b/site/src/modules/management/ManagementSettingsLayout.tsx index abfc197b0b639..8220d6367f0f2 100644 --- a/site/src/modules/management/ManagementSettingsLayout.tsx +++ b/site/src/modules/management/ManagementSettingsLayout.tsx @@ -47,11 +47,15 @@ export const canEditOrganization = ( ); }; -const isManagementRoutePermitted = ( +export const isManagementRoutePermitted = ( locationPath: string, permissions: Permissions, showOrganizations: boolean, ): boolean => { + if (!locationPath.startsWith("/")) { + return false; + } + if (locationPath.startsWith("/organizations")) { return showOrganizations; } @@ -65,7 +69,8 @@ const isManagementRoutePermitted = ( const href = locationPath.replace(/^\/deployment/, ""); if (href === "/" || href === "") { - return true; + const hasAtLeastOnePermission = Object.values(permissions).some((v) => v); + return hasAtLeastOnePermission; } if (href.startsWith("/general")) { return permissions.viewDeploymentValues; @@ -100,6 +105,9 @@ const isManagementRoutePermitted = ( if (href.startsWith("/notifications")) { return permissions.viewNotificationTemplate; } + if (href.startsWith("/oauth2-provider")) { + return permissions.viewExternalAuthConfig; + } return false; }; From 9fc0afabeee59aba2a0bf00c9a257d0c37e97f7f Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Fri, 25 Oct 2024 21:16:47 +0000 Subject: [PATCH 22/30] fix: apply formatting --- site/src/modules/management/ManagementSettingsLayout.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/site/src/modules/management/ManagementSettingsLayout.test.ts b/site/src/modules/management/ManagementSettingsLayout.test.ts index ee0bde062eef6..2dec371550ad2 100644 --- a/site/src/modules/management/ManagementSettingsLayout.test.ts +++ b/site/src/modules/management/ManagementSettingsLayout.test.ts @@ -1,7 +1,7 @@ +import { Permission } from "api/typesGenerated"; import type { Permissions } from "contexts/auth/permissions"; -import { isManagementRoutePermitted } from "./ManagementSettingsLayout"; import { MockNoPermissions, MockPermissions } from "testHelpers/entities"; -import { Permission } from "api/typesGenerated"; +import { isManagementRoutePermitted } from "./ManagementSettingsLayout"; describe(isManagementRoutePermitted.name, () => { describe("General behavior", () => { From 498e7ba162a32b601adf403bce90498c2721ecac Mon Sep 17 00:00:00 2001 From: McKayla Washburn Date: Mon, 28 Oct 2024 18:15:51 +0000 Subject: [PATCH 23/30] break out `DeploymentSettingsProvider` --- .../management/DeploymentSettingsProvider.tsx | 61 +++++++++++++++++ .../management/ManagementSettingsLayout.tsx | 25 +------ .../ExternalAuthSettingsPage.tsx | 8 +-- .../GeneralSettingsPage.tsx | 8 +-- .../NetworkSettingsPage.tsx | 8 +-- .../NotificationEvents.stories.tsx | 6 +- .../NotificationsPage/NotificationEvents.tsx | 8 +-- .../NotificationsPage/NotificationsPage.tsx | 10 +-- .../ObservabilitySettingsPage.tsx | 8 +-- .../SecuritySettingsPage.tsx | 8 +-- .../UserAuthSettingsPage.tsx | 8 +-- site/src/router.tsx | 66 +++++++++++-------- site/src/testHelpers/renderHelpers.tsx | 2 +- site/src/testHelpers/storybook.tsx | 8 ++- 14 files changed, 146 insertions(+), 88 deletions(-) create mode 100644 site/src/modules/management/DeploymentSettingsProvider.tsx diff --git a/site/src/modules/management/DeploymentSettingsProvider.tsx b/site/src/modules/management/DeploymentSettingsProvider.tsx new file mode 100644 index 0000000000000..d863ed475786e --- /dev/null +++ b/site/src/modules/management/DeploymentSettingsProvider.tsx @@ -0,0 +1,61 @@ +import type { DeploymentConfig } from "api/api"; +import { deploymentConfig } from "api/queries/deployment"; +import { ErrorAlert } from "components/Alert/ErrorAlert"; +import { Loader } from "components/Loader/Loader"; +import { useAuthenticated } from "contexts/auth/RequireAuth"; +import { RequirePermission } from "contexts/auth/RequirePermission"; +import { type FC, createContext, useContext } from "react"; +import { useQuery } from "react-query"; +import { Outlet } from "react-router-dom"; + +export const DeploymentSettingsContext = createContext< + DeploymentSettingsValue | undefined +>(undefined); + +type DeploymentSettingsValue = Readonly<{ + deploymentConfig: DeploymentConfig; +}>; + +export const useDeploymentSettings = (): DeploymentSettingsValue => { + const context = useContext(DeploymentSettingsContext); + if (!context) { + throw new Error( + "useDeploymentSettings should be used inside of DeploymentSettingsLayout", + ); + } + + return context; +}; + +const DeploymentSettingsProvider: FC = () => { + const { permissions } = useAuthenticated(); + const deploymentConfigQuery = useQuery(deploymentConfig()); + + // The deployment settings page also contains users, audit logs, groups and + // organizations, so this page must be visible if you can see any of these. + const canViewDeploymentSettingsPage = + permissions.viewDeploymentValues || + permissions.viewAllUsers || + permissions.editAnyOrganization || + permissions.viewAnyAuditLog; + + if (deploymentConfigQuery.error) { + return ; + } + + if (!deploymentConfigQuery.data) { + return ; + } + + return ( + + + + + + ); +}; + +export default DeploymentSettingsProvider; diff --git a/site/src/modules/management/ManagementSettingsLayout.tsx b/site/src/modules/management/ManagementSettingsLayout.tsx index b9fcbc0936e4b..dcc3acf69ef1f 100644 --- a/site/src/modules/management/ManagementSettingsLayout.tsx +++ b/site/src/modules/management/ManagementSettingsLayout.tsx @@ -1,7 +1,4 @@ -import type { DeploymentConfig } from "api/api"; -import { deploymentConfig } from "api/queries/deployment"; import type { AuthorizationResponse, Organization } from "api/typesGenerated"; -import { ErrorAlert } from "components/Alert/ErrorAlert"; import { Loader } from "components/Loader/Loader"; import { Margins } from "components/Margins/Margins"; import { Stack } from "components/Stack/Stack"; @@ -9,7 +6,6 @@ import { useAuthenticated } from "contexts/auth/RequireAuth"; import { RequirePermission } from "contexts/auth/RequirePermission"; import { useDashboard } from "modules/dashboard/useDashboard"; import { type FC, Suspense, createContext, useContext } from "react"; -import { useQuery } from "react-query"; import { Outlet, useParams } from "react-router-dom"; import { Sidebar } from "./Sidebar"; @@ -18,7 +14,6 @@ export const ManagementSettingsContext = createContext< >(undefined); type ManagementSettingsValue = Readonly<{ - deploymentValues: DeploymentConfig; organizations: readonly Organization[]; organization?: Organization; }>; @@ -48,15 +43,8 @@ export const canEditOrganization = ( ); }; -/** - * A multi-org capable settings page layout. - * - * If multi-org is not enabled or licensed, this is the wrong layout to use. - * See DeploySettingsLayoutInner instead. - */ -export const ManagementSettingsLayout: FC = () => { +const ManagementSettingsLayout: FC = () => { const { permissions } = useAuthenticated(); - const deploymentConfigQuery = useQuery(deploymentConfig()); const { organizations } = useDashboard(); const { organization: orgName } = useParams() as { organization?: string; @@ -70,14 +58,6 @@ export const ManagementSettingsLayout: FC = () => { permissions.editAnyOrganization || permissions.viewAnyAuditLog; - if (deploymentConfigQuery.error) { - return ; - } - - if (!deploymentConfigQuery.data) { - return ; - } - const organization = organizations && orgName ? organizations.find((org) => org.name === orgName) @@ -87,7 +67,6 @@ export const ManagementSettingsLayout: FC = () => { { ); }; + +export default ManagementSettingsLayout; diff --git a/site/src/pages/DeploymentSettingsPage/ExternalAuthSettingsPage/ExternalAuthSettingsPage.tsx b/site/src/pages/DeploymentSettingsPage/ExternalAuthSettingsPage/ExternalAuthSettingsPage.tsx index 422cebc7edb93..73a1f6756e312 100644 --- a/site/src/pages/DeploymentSettingsPage/ExternalAuthSettingsPage/ExternalAuthSettingsPage.tsx +++ b/site/src/pages/DeploymentSettingsPage/ExternalAuthSettingsPage/ExternalAuthSettingsPage.tsx @@ -1,12 +1,12 @@ import { Loader } from "components/Loader/Loader"; -import { useManagementSettings } from "modules/management/ManagementSettingsLayout"; import type { FC } from "react"; import { Helmet } from "react-helmet-async"; import { pageTitle } from "utils/page"; import { ExternalAuthSettingsPageView } from "./ExternalAuthSettingsPageView"; +import { useDeploymentSettings } from "modules/management/DeploymentSettingsProvider"; const ExternalAuthSettingsPage: FC = () => { - const { deploymentValues } = useManagementSettings(); + const { deploymentConfig } = useDeploymentSettings(); return ( <> @@ -14,8 +14,8 @@ const ExternalAuthSettingsPage: FC = () => { {pageTitle("External Authentication Settings")} - {deploymentValues ? ( - + {deploymentConfig ? ( + ) : ( )} diff --git a/site/src/pages/DeploymentSettingsPage/GeneralSettingsPage/GeneralSettingsPage.tsx b/site/src/pages/DeploymentSettingsPage/GeneralSettingsPage/GeneralSettingsPage.tsx index 5d3879e195996..a97fddc2f7645 100644 --- a/site/src/pages/DeploymentSettingsPage/GeneralSettingsPage/GeneralSettingsPage.tsx +++ b/site/src/pages/DeploymentSettingsPage/GeneralSettingsPage/GeneralSettingsPage.tsx @@ -3,15 +3,15 @@ import { entitlements } from "api/queries/entitlements"; import { availableExperiments, experiments } from "api/queries/experiments"; import { Loader } from "components/Loader/Loader"; import { useEmbeddedMetadata } from "hooks/useEmbeddedMetadata"; -import { useManagementSettings } from "modules/management/ManagementSettingsLayout"; import type { FC } from "react"; import { Helmet } from "react-helmet-async"; import { useQuery } from "react-query"; import { pageTitle } from "utils/page"; import { GeneralSettingsPageView } from "./GeneralSettingsPageView"; +import { useDeploymentSettings } from "modules/management/DeploymentSettingsProvider"; const GeneralSettingsPage: FC = () => { - const { deploymentValues } = useManagementSettings(); + const { deploymentConfig } = useDeploymentSettings(); const deploymentDAUsQuery = useQuery(deploymentDAUs()); const safeExperimentsQuery = useQuery(availableExperiments()); @@ -30,9 +30,9 @@ const GeneralSettingsPage: FC = () => { {pageTitle("General Settings")} - {deploymentValues ? ( + {deploymentConfig ? ( { - const { deploymentValues } = useManagementSettings(); + const { deploymentConfig } = useDeploymentSettings(); return ( <> @@ -14,8 +14,8 @@ const NetworkSettingsPage: FC = () => { {pageTitle("Network Settings")} - {deploymentValues ? ( - + {deploymentConfig ? ( + ) : ( )} diff --git a/site/src/pages/DeploymentSettingsPage/NotificationsPage/NotificationEvents.stories.tsx b/site/src/pages/DeploymentSettingsPage/NotificationsPage/NotificationEvents.stories.tsx index c2e8479a26f8c..61a1eddcd1a78 100644 --- a/site/src/pages/DeploymentSettingsPage/NotificationsPage/NotificationEvents.stories.tsx +++ b/site/src/pages/DeploymentSettingsPage/NotificationsPage/NotificationEvents.stories.tsx @@ -14,7 +14,7 @@ const meta: Meta = { defaultMethod: "smtp", availableMethods: ["smtp", "webhook"], templatesByGroup: selectTemplatesByGroup(MockNotificationTemplates), - deploymentValues: baseMeta.parameters.deploymentValues, + deploymentConfig: baseMeta.parameters.deploymentValues, }, ...baseMeta, }; @@ -25,7 +25,7 @@ type Story = StoryObj; export const SMTPNotConfigured: Story = { args: { - deploymentValues: { + deploymentConfig: { notifications: { webhook: { endpoint: "https://example.com", @@ -40,7 +40,7 @@ export const SMTPNotConfigured: Story = { export const WebhookNotConfigured: Story = { args: { - deploymentValues: { + deploymentConfig: { notifications: { webhook: { endpoint: "", diff --git a/site/src/pages/DeploymentSettingsPage/NotificationsPage/NotificationEvents.tsx b/site/src/pages/DeploymentSettingsPage/NotificationsPage/NotificationEvents.tsx index 191e2eda6958e..38c36fc52c044 100644 --- a/site/src/pages/DeploymentSettingsPage/NotificationsPage/NotificationEvents.tsx +++ b/site/src/pages/DeploymentSettingsPage/NotificationsPage/NotificationEvents.tsx @@ -31,20 +31,20 @@ type NotificationEventsProps = { defaultMethod: NotificationMethod; availableMethods: NotificationMethod[]; templatesByGroup: ReturnType; - deploymentValues: DeploymentValues; + deploymentConfig: DeploymentValues; }; export const NotificationEvents: FC = ({ defaultMethod, availableMethods, templatesByGroup, - deploymentValues, + deploymentConfig, }) => { // Webhook const hasWebhookNotifications = Object.values(templatesByGroup) .flat() .some((t) => t.method === "webhook"); - const webhookValues = deploymentValues.notifications?.webhook ?? {}; + const webhookValues = deploymentConfig.notifications?.webhook ?? {}; const isWebhookConfigured = requiredFieldsArePresent(webhookValues, [ "endpoint", ]); @@ -53,7 +53,7 @@ export const NotificationEvents: FC = ({ const hasSMTPNotifications = Object.values(templatesByGroup) .flat() .some((t) => t.method === "smtp"); - const smtpValues = deploymentValues.notifications?.email ?? {}; + const smtpValues = deploymentConfig.notifications?.email ?? {}; const isSMTPConfigured = requiredFieldsArePresent(smtpValues, [ "smarthost", "from", diff --git a/site/src/pages/DeploymentSettingsPage/NotificationsPage/NotificationsPage.tsx b/site/src/pages/DeploymentSettingsPage/NotificationsPage/NotificationsPage.tsx index d43c8c3a841a6..193bde290f748 100644 --- a/site/src/pages/DeploymentSettingsPage/NotificationsPage/NotificationsPage.tsx +++ b/site/src/pages/DeploymentSettingsPage/NotificationsPage/NotificationsPage.tsx @@ -6,7 +6,6 @@ import { } from "api/queries/notifications"; import { Loader } from "components/Loader/Loader"; import { TabLink, Tabs, TabsList } from "components/Tabs/Tabs"; -import { useManagementSettings } from "modules/management/ManagementSettingsLayout"; import { castNotificationMethod } from "modules/notifications/utils"; import { Section } from "pages/UserSettingsPage/Section"; import type { FC } from "react"; @@ -17,10 +16,11 @@ import { deploymentGroupHasParent } from "utils/deployOptions"; import { pageTitle } from "utils/page"; import OptionsTable from "../OptionsTable"; import { NotificationEvents } from "./NotificationEvents"; +import { useDeploymentSettings } from "modules/management/DeploymentSettingsProvider"; export const NotificationsPage: FC = () => { const [searchParams] = useSearchParams(); - const { deploymentValues } = useManagementSettings(); + const { deploymentConfig } = useDeploymentSettings(); const [templatesByGroup, dispatchMethods] = useQueries({ queries: [ { @@ -31,7 +31,7 @@ export const NotificationsPage: FC = () => { ], }); const ready = - templatesByGroup.data && dispatchMethods.data && deploymentValues; + templatesByGroup.data && dispatchMethods.data && deploymentConfig; const tab = searchParams.get("tab") || "events"; return ( @@ -61,7 +61,7 @@ export const NotificationsPage: FC = () => { tab === "events" ? ( { /> ) : ( + options={deploymentConfig?.options.filter((o) => deploymentGroupHasParent(o.group, "Notifications"), )} /> diff --git a/site/src/pages/DeploymentSettingsPage/ObservabilitySettingsPage/ObservabilitySettingsPage.tsx b/site/src/pages/DeploymentSettingsPage/ObservabilitySettingsPage/ObservabilitySettingsPage.tsx index 1ea1a2d19ef82..e50d01bdf44ea 100644 --- a/site/src/pages/DeploymentSettingsPage/ObservabilitySettingsPage/ObservabilitySettingsPage.tsx +++ b/site/src/pages/DeploymentSettingsPage/ObservabilitySettingsPage/ObservabilitySettingsPage.tsx @@ -1,14 +1,14 @@ import { Loader } from "components/Loader/Loader"; import { useDashboard } from "modules/dashboard/useDashboard"; import { useFeatureVisibility } from "modules/dashboard/useFeatureVisibility"; -import { useManagementSettings } from "modules/management/ManagementSettingsLayout"; import type { FC } from "react"; import { Helmet } from "react-helmet-async"; import { pageTitle } from "utils/page"; import { ObservabilitySettingsPageView } from "./ObservabilitySettingsPageView"; +import { useDeploymentSettings } from "modules/management/DeploymentSettingsProvider"; const ObservabilitySettingsPage: FC = () => { - const { deploymentValues } = useManagementSettings(); + const { deploymentConfig } = useDeploymentSettings(); const { entitlements } = useDashboard(); const { multiple_organizations: hasPremiumLicense } = useFeatureVisibility(); @@ -18,9 +18,9 @@ const ObservabilitySettingsPage: FC = () => { {pageTitle("Observability Settings")} - {deploymentValues ? ( + {deploymentConfig ? ( diff --git a/site/src/pages/DeploymentSettingsPage/SecuritySettingsPage/SecuritySettingsPage.tsx b/site/src/pages/DeploymentSettingsPage/SecuritySettingsPage/SecuritySettingsPage.tsx index 2a296fc9d22d2..4e6c75f0bac81 100644 --- a/site/src/pages/DeploymentSettingsPage/SecuritySettingsPage/SecuritySettingsPage.tsx +++ b/site/src/pages/DeploymentSettingsPage/SecuritySettingsPage/SecuritySettingsPage.tsx @@ -1,13 +1,13 @@ import { Loader } from "components/Loader/Loader"; import { useDashboard } from "modules/dashboard/useDashboard"; -import { useManagementSettings } from "modules/management/ManagementSettingsLayout"; import type { FC } from "react"; import { Helmet } from "react-helmet-async"; import { pageTitle } from "utils/page"; import { SecuritySettingsPageView } from "./SecuritySettingsPageView"; +import { useDeploymentSettings } from "modules/management/DeploymentSettingsProvider"; const SecuritySettingsPage: FC = () => { - const { deploymentValues } = useManagementSettings(); + const { deploymentConfig } = useDeploymentSettings(); const { entitlements } = useDashboard(); return ( @@ -16,9 +16,9 @@ const SecuritySettingsPage: FC = () => { {pageTitle("Security Settings")} - {deploymentValues ? ( + {deploymentConfig ? ( ) : ( diff --git a/site/src/pages/DeploymentSettingsPage/UserAuthSettingsPage/UserAuthSettingsPage.tsx b/site/src/pages/DeploymentSettingsPage/UserAuthSettingsPage/UserAuthSettingsPage.tsx index b6382f5a54f99..61dbad7959359 100644 --- a/site/src/pages/DeploymentSettingsPage/UserAuthSettingsPage/UserAuthSettingsPage.tsx +++ b/site/src/pages/DeploymentSettingsPage/UserAuthSettingsPage/UserAuthSettingsPage.tsx @@ -1,12 +1,12 @@ import { Loader } from "components/Loader/Loader"; -import { useManagementSettings } from "modules/management/ManagementSettingsLayout"; import type { FC } from "react"; import { Helmet } from "react-helmet-async"; import { pageTitle } from "utils/page"; import { UserAuthSettingsPageView } from "./UserAuthSettingsPageView"; +import { useDeploymentSettings } from "modules/management/DeploymentSettingsProvider"; const UserAuthSettingsPage: FC = () => { - const { deploymentValues } = useManagementSettings(); + const { deploymentConfig } = useDeploymentSettings(); return ( <> @@ -14,8 +14,8 @@ const UserAuthSettingsPage: FC = () => { {pageTitle("User Authentication Settings")} - {deploymentValues ? ( - + {deploymentConfig ? ( + ) : ( )} diff --git a/site/src/router.tsx b/site/src/router.tsx index 2531c823b9f48..56d2437d30eba 100644 --- a/site/src/router.tsx +++ b/site/src/router.tsx @@ -10,7 +10,6 @@ import { import { Loader } from "./components/Loader/Loader"; import { RequireAuth } from "./contexts/auth/RequireAuth"; import { DashboardLayout } from "./modules/dashboard/DashboardLayout"; -import { ManagementSettingsLayout } from "./modules/management/ManagementSettingsLayout"; import AuditPage from "./pages/AuditPage/AuditPage"; import { HealthLayout } from "./pages/HealthPage/HealthLayout"; import LoginPage from "./pages/LoginPage/LoginPage"; @@ -28,6 +27,12 @@ import WorkspacesPage from "./pages/WorkspacesPage/WorkspacesPage"; // - Pages that are secondary, not in the main navigation or not usually accessed // - Pages that use heavy dependencies like charts or time libraries const NotFoundPage = lazy(() => import("./pages/404Page/404Page")); +const ManagementSettingsLayout = lazy( + () => import("./modules/management/ManagementSettingsLayout"), +); +const DeploymentSettingsProvider = lazy( + () => import("./modules/management/DeploymentSettingsProvider"), +); const CliAuthenticationPage = lazy( () => import("./pages/CliAuthPage/CliAuthPage"), ); @@ -427,39 +432,46 @@ export const router = createBrowserRouter( }> - } /> - } /> - } /> - } /> - } - /> - } /> - } /> - } /> - } - /> + }> + } /> + } /> + } /> + } /> + } + /> + } /> + } /> + } /> + } + /> - - } /> - - } /> - } /> - } /> + + } /> + + } /> + } /> + } /> + + + } + /> + + } + /> - } /> } /> } /> {groupsRouter()} - } - /> }> diff --git a/site/src/testHelpers/renderHelpers.tsx b/site/src/testHelpers/renderHelpers.tsx index f093adb1cfb4a..46ae893927801 100644 --- a/site/src/testHelpers/renderHelpers.tsx +++ b/site/src/testHelpers/renderHelpers.tsx @@ -9,7 +9,7 @@ import { ThemeProvider } from "contexts/ThemeProvider"; import { RequireAuth } from "contexts/auth/RequireAuth"; import { DashboardLayout } from "modules/dashboard/DashboardLayout"; import type { DashboardProvider } from "modules/dashboard/DashboardProvider"; -import { ManagementSettingsLayout } from "modules/management/ManagementSettingsLayout"; +import ManagementSettingsLayout from "modules/management/ManagementSettingsLayout"; import { TemplateSettingsLayout } from "pages/TemplateSettingsPage/TemplateSettingsLayout"; import { WorkspaceSettingsLayout } from "pages/WorkspaceSettingsPage/WorkspaceSettingsLayout"; import type { ReactNode } from "react"; diff --git a/site/src/testHelpers/storybook.tsx b/site/src/testHelpers/storybook.tsx index a76e1230205fc..78a97ce163ed5 100644 --- a/site/src/testHelpers/storybook.tsx +++ b/site/src/testHelpers/storybook.tsx @@ -16,6 +16,7 @@ import { MockDeploymentConfig, MockEntitlements, } from "./entities"; +import { DeploymentSettingsContext } from "modules/management/DeploymentSettingsProvider"; export const withDashboardProvider = ( Story: FC, @@ -131,12 +132,15 @@ export const withManagementSettingsProvider = (Story: FC) => { return ( - + + + ); }; From 234c6060035f9532788991502673b6f1f8c4d2b3 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Mon, 28 Oct 2024 22:05:23 +0000 Subject: [PATCH 24/30] fix: apply changes from previous PR --- site/e2e/global.setup.ts | 1 + site/jest.setup.ts | 8 ++--- site/src/contexts/auth/permissions.tsx | 28 +++++++++++++--- .../management/DeploymentSettingsProvider.tsx | 5 ++- .../management/ManagementSettingsLayout.tsx | 2 +- .../management/SidebarView.stories.tsx | 3 +- site/src/modules/management/SidebarView.tsx | 33 ++++++++++--------- .../ExternalAuthSettingsPage.tsx | 7 +--- .../GeneralSettingsPage.tsx | 21 +++++------- .../NetworkSettingsPage.tsx | 7 +--- .../NotificationsPage/NotificationsPage.tsx | 17 +++++----- .../ObservabilitySettingsPage.tsx | 16 +++------ .../SecuritySettingsPage.tsx | 13 +++----- .../UserAuthSettingsPage.tsx | 7 +--- .../OrganizationSettingsPage.tsx | 13 +++++--- site/src/router.tsx | 32 +++++++++--------- site/src/testHelpers/entities.ts | 25 ++++++++++++++ 17 files changed, 130 insertions(+), 108 deletions(-) diff --git a/site/e2e/global.setup.ts b/site/e2e/global.setup.ts index 6eafd2886de37..f39a2d475804e 100644 --- a/site/e2e/global.setup.ts +++ b/site/e2e/global.setup.ts @@ -35,6 +35,7 @@ test("setup deployment", async ({ page }) => { expect(constants.license.split(".").length).toBe(3); // otherwise it's invalid await page.goto("/deployment/licenses", { waitUntil: "domcontentloaded" }); + await expect(page).toHaveTitle("License Settings - Coder"); await page.getByText("Add a license").click(); await page.getByRole("textbox").fill(constants.license); diff --git a/site/jest.setup.ts b/site/jest.setup.ts index 7a06ebba2592f..7d4b6f0772bc4 100644 --- a/site/jest.setup.ts +++ b/site/jest.setup.ts @@ -1,7 +1,7 @@ import "@testing-library/jest-dom"; import "jest-location-mock"; import { cleanup } from "@testing-library/react"; -import crypto from "crypto"; +import crypto from "node:crypto"; import { useMemo } from "react"; import type { Region } from "api/typesGenerated"; import type { ProxyLatencyReport } from "contexts/useProxyLatency"; @@ -48,9 +48,7 @@ global.ResizeObserver = require("resize-observer-polyfill"); // Polyfill the getRandomValues that is used on utils/random.ts Object.defineProperty(global.self, "crypto", { value: { - getRandomValues: function (buffer: Buffer) { - return crypto.randomFillSync(buffer); - }, + getRandomValues: crypto.randomFillSync, }, }); @@ -72,5 +70,5 @@ afterEach(() => { // Clean up after the tests are finished. afterAll(() => server.close()); -// This is needed because we are compiling under `--isolatedModules` +// biome-ignore lint/complexity/noUselessEmptyExport: This is needed because we are compiling under `--isolatedModules` export {}; diff --git a/site/src/contexts/auth/permissions.tsx b/site/src/contexts/auth/permissions.tsx index 0c89d81686d2f..d2de7864874f0 100644 --- a/site/src/contexts/auth/permissions.tsx +++ b/site/src/contexts/auth/permissions.tsx @@ -1,3 +1,5 @@ +import type { AuthorizationCheck } from "api/typesGenerated"; + export const checks = { viewAllUsers: "viewAllUsers", updateUsers: "updateUsers", @@ -11,13 +13,20 @@ export const checks = { viewUpdateCheck: "viewUpdateCheck", viewExternalAuthConfig: "viewExternalAuthConfig", viewDeploymentStats: "viewDeploymentStats", + readWorkspaceProxies: "readWorkspaceProxies", editWorkspaceProxies: "editWorkspaceProxies", createOrganization: "createOrganization", editAnyOrganization: "editAnyOrganization", viewAnyGroup: "viewAnyGroup", createGroup: "createGroup", viewAllLicenses: "viewAllLicenses", -} as const; + viewNotificationTemplate: "viewNotificationTemplate", +} as const satisfies Record; + +// Type expression seems a little redundant (`keyof typeof checks` has the same +// result), just because each key-value pair is currently symmetrical; this may +// change down the line +type PermissionValue = (typeof checks)[keyof typeof checks]; export const permissionsToCheck = { [checks.viewAllUsers]: { @@ -94,6 +103,12 @@ export const permissionsToCheck = { }, action: "read", }, + [checks.readWorkspaceProxies]: { + object: { + resource_type: "workspace_proxy", + }, + action: "read", + }, [checks.editWorkspaceProxies]: { object: { resource_type: "workspace_proxy", @@ -116,7 +131,6 @@ export const permissionsToCheck = { [checks.viewAnyGroup]: { object: { resource_type: "group", - org_id: "any", }, action: "read", }, @@ -132,6 +146,12 @@ export const permissionsToCheck = { }, action: "read", }, -} as const; + [checks.viewNotificationTemplate]: { + object: { + resource_type: "notification_template", + }, + action: "read", + }, +} as const satisfies Record; -export type Permissions = Record; +export type Permissions = Record; diff --git a/site/src/modules/management/DeploymentSettingsProvider.tsx b/site/src/modules/management/DeploymentSettingsProvider.tsx index d863ed475786e..c9f6cd5f4a8ce 100644 --- a/site/src/modules/management/DeploymentSettingsProvider.tsx +++ b/site/src/modules/management/DeploymentSettingsProvider.tsx @@ -20,7 +20,7 @@ export const useDeploymentSettings = (): DeploymentSettingsValue => { const context = useContext(DeploymentSettingsContext); if (!context) { throw new Error( - "useDeploymentSettings should be used inside of DeploymentSettingsLayout", + `${useDeploymentSettings.name} should be used inside of ${DeploymentSettingsProvider.name}`, ); } @@ -39,6 +39,9 @@ const DeploymentSettingsProvider: FC = () => { permissions.editAnyOrganization || permissions.viewAnyAuditLog; + // Not a huge problem to unload the content in the event of an error, + // because the sidebar rendering isn't tied to this. Even if the user hits + // a 403 error, they'll still have navigation options if (deploymentConfigQuery.error) { return ; } diff --git a/site/src/modules/management/ManagementSettingsLayout.tsx b/site/src/modules/management/ManagementSettingsLayout.tsx index dcc3acf69ef1f..0cb313f0e53b9 100644 --- a/site/src/modules/management/ManagementSettingsLayout.tsx +++ b/site/src/modules/management/ManagementSettingsLayout.tsx @@ -74,7 +74,7 @@ const ManagementSettingsLayout: FC = () => { -
+
}> diff --git a/site/src/modules/management/SidebarView.stories.tsx b/site/src/modules/management/SidebarView.stories.tsx index 2ddcf7750bc8d..6ffe4480261c9 100644 --- a/site/src/modules/management/SidebarView.stories.tsx +++ b/site/src/modules/management/SidebarView.stories.tsx @@ -1,5 +1,6 @@ import type { Meta, StoryObj } from "@storybook/react"; import { + MockNoPermissions, MockOrganization, MockOrganization2, MockPermissions, @@ -96,7 +97,7 @@ export const NoDeploymentValues: Story = { export const NoPermissions: Story = { args: { - permissions: {}, + permissions: MockNoPermissions, }, }; diff --git a/site/src/modules/management/SidebarView.tsx b/site/src/modules/management/SidebarView.tsx index b4099a4dd7815..970ba531d6ca4 100644 --- a/site/src/modules/management/SidebarView.tsx +++ b/site/src/modules/management/SidebarView.tsx @@ -12,9 +12,9 @@ import { Loader } from "components/Loader/Loader"; import { Sidebar as BaseSidebar } from "components/Sidebar/Sidebar"; import { Stack } from "components/Stack/Stack"; import { UserAvatar } from "components/UserAvatar/UserAvatar"; +import type { Permissions } from "contexts/auth/permissions"; import { type ClassName, useClassName } from "hooks/useClassName"; import { useDashboard } from "modules/dashboard/useDashboard"; -import { linkToUsers } from "modules/navigation"; import type { FC, ReactNode } from "react"; import { Link, NavLink } from "react-router-dom"; @@ -30,7 +30,7 @@ interface SidebarProps { /** Organizations and their permissions or undefined if still fetching. */ organizations: OrganizationWithPermissions[] | undefined; /** Site-wide permissions. */ - permissions: AuthorizationResponse; + permissions: Permissions; } /** @@ -72,7 +72,7 @@ interface DeploymentSettingsNavigationProps { /** Whether a deployment setting page is being viewed. */ active: boolean; /** Site-wide permissions. */ - permissions: AuthorizationResponse; + permissions: Permissions; } /** @@ -130,10 +130,11 @@ const DeploymentSettingsNavigation: FC = ({ {permissions.viewDeploymentValues && ( Network )} - {/* All users can view workspace regions. */} - - Workspace Proxies - + {permissions.readWorkspaceProxies && ( + + Workspace Proxies + + )} {permissions.viewDeploymentValues && ( Security )} @@ -145,12 +146,14 @@ const DeploymentSettingsNavigation: FC = ({ {permissions.viewAllUsers && ( Users )} - - - Notifications - - - + {permissions.viewNotificationTemplate && ( + + + Notifications + + + + )} )} @@ -167,7 +170,7 @@ interface OrganizationsSettingsNavigationProps { /** Organizations and their permissions or undefined if still fetching. */ organizations: OrganizationWithPermissions[] | undefined; /** Site-wide permissions. */ - permissions: AuthorizationResponse; + permissions: Permissions; } /** @@ -241,8 +244,6 @@ interface OrganizationSettingsNavigationProps { const OrganizationSettingsNavigation: FC< OrganizationSettingsNavigationProps > = ({ active, organization }) => { - const { experiments } = useDashboard(); - return ( <> { {pageTitle("External Authentication Settings")} - - {deploymentConfig ? ( - - ) : ( - - )} + ); }; diff --git a/site/src/pages/DeploymentSettingsPage/GeneralSettingsPage/GeneralSettingsPage.tsx b/site/src/pages/DeploymentSettingsPage/GeneralSettingsPage/GeneralSettingsPage.tsx index a97fddc2f7645..dd68543a1af5f 100644 --- a/site/src/pages/DeploymentSettingsPage/GeneralSettingsPage/GeneralSettingsPage.tsx +++ b/site/src/pages/DeploymentSettingsPage/GeneralSettingsPage/GeneralSettingsPage.tsx @@ -1,7 +1,6 @@ import { deploymentDAUs } from "api/queries/deployment"; import { entitlements } from "api/queries/entitlements"; import { availableExperiments, experiments } from "api/queries/experiments"; -import { Loader } from "components/Loader/Loader"; import { useEmbeddedMetadata } from "hooks/useEmbeddedMetadata"; import type { FC } from "react"; import { Helmet } from "react-helmet-async"; @@ -30,18 +29,14 @@ const GeneralSettingsPage: FC = () => { {pageTitle("General Settings")} - {deploymentConfig ? ( - - ) : ( - - )} + ); }; diff --git a/site/src/pages/DeploymentSettingsPage/NetworkSettingsPage/NetworkSettingsPage.tsx b/site/src/pages/DeploymentSettingsPage/NetworkSettingsPage/NetworkSettingsPage.tsx index 1baa586f28a35..a53ff66f0631d 100644 --- a/site/src/pages/DeploymentSettingsPage/NetworkSettingsPage/NetworkSettingsPage.tsx +++ b/site/src/pages/DeploymentSettingsPage/NetworkSettingsPage/NetworkSettingsPage.tsx @@ -13,12 +13,7 @@ const NetworkSettingsPage: FC = () => { {pageTitle("Network Settings")} - - {deploymentConfig ? ( - - ) : ( - - )} + ); }; diff --git a/site/src/pages/DeploymentSettingsPage/NotificationsPage/NotificationsPage.tsx b/site/src/pages/DeploymentSettingsPage/NotificationsPage/NotificationsPage.tsx index 193bde290f748..0e0c1d6cb435d 100644 --- a/site/src/pages/DeploymentSettingsPage/NotificationsPage/NotificationsPage.tsx +++ b/site/src/pages/DeploymentSettingsPage/NotificationsPage/NotificationsPage.tsx @@ -11,15 +11,14 @@ import { Section } from "pages/UserSettingsPage/Section"; import type { FC } from "react"; import { Helmet } from "react-helmet-async"; import { useQueries } from "react-query"; -import { useSearchParams } from "react-router-dom"; import { deploymentGroupHasParent } from "utils/deployOptions"; import { pageTitle } from "utils/page"; import OptionsTable from "../OptionsTable"; import { NotificationEvents } from "./NotificationEvents"; import { useDeploymentSettings } from "modules/management/DeploymentSettingsProvider"; +import { useSearchParamsKey } from "hooks/useSearchParamsKey"; export const NotificationsPage: FC = () => { - const [searchParams] = useSearchParams(); const { deploymentConfig } = useDeploymentSettings(); const [templatesByGroup, dispatchMethods] = useQueries({ queries: [ @@ -30,10 +29,12 @@ export const NotificationsPage: FC = () => { notificationDispatchMethods(), ], }); - const ready = - templatesByGroup.data && dispatchMethods.data && deploymentConfig; - const tab = searchParams.get("tab") || "events"; + const tabState = useSearchParamsKey({ + key: "tab", + defaultValue: "events", + }); + const ready = !!(templatesByGroup.data && dispatchMethods.data); return ( <> @@ -45,7 +46,7 @@ export const NotificationsPage: FC = () => { layout="fluid" featureStage={"beta"} > - + Events @@ -58,7 +59,7 @@ export const NotificationsPage: FC = () => {
{ready ? ( - tab === "events" ? ( + tabState.value === "events" ? ( { /> ) : ( + options={deploymentConfig.options.filter((o) => deploymentGroupHasParent(o.group, "Notifications"), )} /> diff --git a/site/src/pages/DeploymentSettingsPage/ObservabilitySettingsPage/ObservabilitySettingsPage.tsx b/site/src/pages/DeploymentSettingsPage/ObservabilitySettingsPage/ObservabilitySettingsPage.tsx index e50d01bdf44ea..f116e241399d9 100644 --- a/site/src/pages/DeploymentSettingsPage/ObservabilitySettingsPage/ObservabilitySettingsPage.tsx +++ b/site/src/pages/DeploymentSettingsPage/ObservabilitySettingsPage/ObservabilitySettingsPage.tsx @@ -1,4 +1,3 @@ -import { Loader } from "components/Loader/Loader"; import { useDashboard } from "modules/dashboard/useDashboard"; import { useFeatureVisibility } from "modules/dashboard/useFeatureVisibility"; import type { FC } from "react"; @@ -17,16 +16,11 @@ const ObservabilitySettingsPage: FC = () => { {pageTitle("Observability Settings")} - - {deploymentConfig ? ( - - ) : ( - - )} + ); }; diff --git a/site/src/pages/DeploymentSettingsPage/SecuritySettingsPage/SecuritySettingsPage.tsx b/site/src/pages/DeploymentSettingsPage/SecuritySettingsPage/SecuritySettingsPage.tsx index 4e6c75f0bac81..4317af0293e5d 100644 --- a/site/src/pages/DeploymentSettingsPage/SecuritySettingsPage/SecuritySettingsPage.tsx +++ b/site/src/pages/DeploymentSettingsPage/SecuritySettingsPage/SecuritySettingsPage.tsx @@ -15,15 +15,10 @@ const SecuritySettingsPage: FC = () => { {pageTitle("Security Settings")} - - {deploymentConfig ? ( - - ) : ( - - )} + ); }; diff --git a/site/src/pages/DeploymentSettingsPage/UserAuthSettingsPage/UserAuthSettingsPage.tsx b/site/src/pages/DeploymentSettingsPage/UserAuthSettingsPage/UserAuthSettingsPage.tsx index 61dbad7959359..7a1f5ca5961ce 100644 --- a/site/src/pages/DeploymentSettingsPage/UserAuthSettingsPage/UserAuthSettingsPage.tsx +++ b/site/src/pages/DeploymentSettingsPage/UserAuthSettingsPage/UserAuthSettingsPage.tsx @@ -13,12 +13,7 @@ const UserAuthSettingsPage: FC = () => { {pageTitle("User Authentication Settings")} - - {deploymentConfig ? ( - - ) : ( - - )} + ); }; diff --git a/site/src/pages/ManagementSettingsPage/OrganizationSettingsPage.tsx b/site/src/pages/ManagementSettingsPage/OrganizationSettingsPage.tsx index 1d11c85a605ae..63dd6a2d47ce4 100644 --- a/site/src/pages/ManagementSettingsPage/OrganizationSettingsPage.tsx +++ b/site/src/pages/ManagementSettingsPage/OrganizationSettingsPage.tsx @@ -55,12 +55,17 @@ const OrganizationSettingsPage: FC = () => { // Redirect /organizations => /organizations/default-org, or if they cannot edit // the default org, then the first org they can edit, if any. if (!organizationName) { + // .find will stop at the first match found; make sure default + // organizations are placed first const editableOrg = [...organizations] .sort((a, b) => { - // Prefer default org (it may not be first). - // JavaScript will happily subtract booleans, but use numbers to keep - // the compiler happy. - return (b.is_default ? 1 : 0) - (a.is_default ? 1 : 0); + if (a.is_default && !b.is_default) { + return -1; + } + if (b.is_default && !a.is_default) { + return 1; + } + return 0; }) .find((org) => canEditOrganization(permissions[org.id])); if (editableOrg) { diff --git a/site/src/router.tsx b/site/src/router.tsx index 56d2437d30eba..c9d8736979c34 100644 --- a/site/src/router.tsx +++ b/site/src/router.tsx @@ -434,14 +434,11 @@ export const router = createBrowserRouter( }> }> } /> - } /> - } /> } /> } /> - } /> } /> } /> } /> - - } /> - - } /> - } /> - } /> - - - - } - /> - } /> + + } /> + } /> + + } /> + } /> + + } /> + + } /> + } /> + } /> + + + } /> } /> {groupsRouter()} diff --git a/site/src/testHelpers/entities.ts b/site/src/testHelpers/entities.ts index 0db6e80d435d6..1593790e9792d 100644 --- a/site/src/testHelpers/entities.ts +++ b/site/src/testHelpers/entities.ts @@ -2766,12 +2766,37 @@ export const MockPermissions: Permissions = { viewUpdateCheck: true, viewDeploymentStats: true, viewExternalAuthConfig: true, + readWorkspaceProxies: true, editWorkspaceProxies: true, createOrganization: true, editAnyOrganization: true, viewAnyGroup: true, createGroup: true, viewAllLicenses: true, + viewNotificationTemplate: true, +}; + +export const MockNoPermissions: Permissions = { + createTemplates: false, + createUser: false, + deleteTemplates: false, + updateTemplates: false, + viewAllUsers: false, + updateUsers: false, + viewAnyAuditLog: false, + viewDeploymentValues: false, + editDeploymentValues: false, + viewUpdateCheck: false, + viewDeploymentStats: false, + viewExternalAuthConfig: false, + readWorkspaceProxies: false, + editWorkspaceProxies: false, + createOrganization: false, + editAnyOrganization: false, + viewAnyGroup: false, + createGroup: false, + viewAllLicenses: false, + viewNotificationTemplate: false, }; export const MockDeploymentConfig: DeploymentConfig = { From 4496a75c578fd28e0c8c84d9331096c10ac8d438 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Mon, 28 Oct 2024 22:15:00 +0000 Subject: [PATCH 25/30] fix: apply formatting --- .../ExternalAuthSettingsPage/ExternalAuthSettingsPage.tsx | 2 +- .../GeneralSettingsPage/GeneralSettingsPage.tsx | 2 +- .../NetworkSettingsPage/NetworkSettingsPage.tsx | 2 +- .../NotificationsPage/NotificationsPage.tsx | 4 ++-- .../ObservabilitySettingsPage/ObservabilitySettingsPage.tsx | 2 +- .../SecuritySettingsPage/SecuritySettingsPage.tsx | 2 +- .../UserAuthSettingsPage/UserAuthSettingsPage.tsx | 2 +- site/src/testHelpers/storybook.tsx | 2 +- 8 files changed, 9 insertions(+), 9 deletions(-) diff --git a/site/src/pages/DeploymentSettingsPage/ExternalAuthSettingsPage/ExternalAuthSettingsPage.tsx b/site/src/pages/DeploymentSettingsPage/ExternalAuthSettingsPage/ExternalAuthSettingsPage.tsx index 6817158c999bf..27edefa229b2f 100644 --- a/site/src/pages/DeploymentSettingsPage/ExternalAuthSettingsPage/ExternalAuthSettingsPage.tsx +++ b/site/src/pages/DeploymentSettingsPage/ExternalAuthSettingsPage/ExternalAuthSettingsPage.tsx @@ -1,9 +1,9 @@ import { Loader } from "components/Loader/Loader"; +import { useDeploymentSettings } from "modules/management/DeploymentSettingsProvider"; import type { FC } from "react"; import { Helmet } from "react-helmet-async"; import { pageTitle } from "utils/page"; import { ExternalAuthSettingsPageView } from "./ExternalAuthSettingsPageView"; -import { useDeploymentSettings } from "modules/management/DeploymentSettingsProvider"; const ExternalAuthSettingsPage: FC = () => { const { deploymentConfig } = useDeploymentSettings(); diff --git a/site/src/pages/DeploymentSettingsPage/GeneralSettingsPage/GeneralSettingsPage.tsx b/site/src/pages/DeploymentSettingsPage/GeneralSettingsPage/GeneralSettingsPage.tsx index dd68543a1af5f..2b094cbf89b26 100644 --- a/site/src/pages/DeploymentSettingsPage/GeneralSettingsPage/GeneralSettingsPage.tsx +++ b/site/src/pages/DeploymentSettingsPage/GeneralSettingsPage/GeneralSettingsPage.tsx @@ -2,12 +2,12 @@ import { deploymentDAUs } from "api/queries/deployment"; import { entitlements } from "api/queries/entitlements"; import { availableExperiments, experiments } from "api/queries/experiments"; import { useEmbeddedMetadata } from "hooks/useEmbeddedMetadata"; +import { useDeploymentSettings } from "modules/management/DeploymentSettingsProvider"; import type { FC } from "react"; import { Helmet } from "react-helmet-async"; import { useQuery } from "react-query"; import { pageTitle } from "utils/page"; import { GeneralSettingsPageView } from "./GeneralSettingsPageView"; -import { useDeploymentSettings } from "modules/management/DeploymentSettingsProvider"; const GeneralSettingsPage: FC = () => { const { deploymentConfig } = useDeploymentSettings(); diff --git a/site/src/pages/DeploymentSettingsPage/NetworkSettingsPage/NetworkSettingsPage.tsx b/site/src/pages/DeploymentSettingsPage/NetworkSettingsPage/NetworkSettingsPage.tsx index a53ff66f0631d..ec77bb95e5241 100644 --- a/site/src/pages/DeploymentSettingsPage/NetworkSettingsPage/NetworkSettingsPage.tsx +++ b/site/src/pages/DeploymentSettingsPage/NetworkSettingsPage/NetworkSettingsPage.tsx @@ -1,9 +1,9 @@ import { Loader } from "components/Loader/Loader"; +import { useDeploymentSettings } from "modules/management/DeploymentSettingsProvider"; import type { FC } from "react"; import { Helmet } from "react-helmet-async"; import { pageTitle } from "utils/page"; import { NetworkSettingsPageView } from "./NetworkSettingsPageView"; -import { useDeploymentSettings } from "modules/management/DeploymentSettingsProvider"; const NetworkSettingsPage: FC = () => { const { deploymentConfig } = useDeploymentSettings(); diff --git a/site/src/pages/DeploymentSettingsPage/NotificationsPage/NotificationsPage.tsx b/site/src/pages/DeploymentSettingsPage/NotificationsPage/NotificationsPage.tsx index 0e0c1d6cb435d..23f8e6b42651e 100644 --- a/site/src/pages/DeploymentSettingsPage/NotificationsPage/NotificationsPage.tsx +++ b/site/src/pages/DeploymentSettingsPage/NotificationsPage/NotificationsPage.tsx @@ -6,6 +6,8 @@ import { } from "api/queries/notifications"; import { Loader } from "components/Loader/Loader"; import { TabLink, Tabs, TabsList } from "components/Tabs/Tabs"; +import { useSearchParamsKey } from "hooks/useSearchParamsKey"; +import { useDeploymentSettings } from "modules/management/DeploymentSettingsProvider"; import { castNotificationMethod } from "modules/notifications/utils"; import { Section } from "pages/UserSettingsPage/Section"; import type { FC } from "react"; @@ -15,8 +17,6 @@ import { deploymentGroupHasParent } from "utils/deployOptions"; import { pageTitle } from "utils/page"; import OptionsTable from "../OptionsTable"; import { NotificationEvents } from "./NotificationEvents"; -import { useDeploymentSettings } from "modules/management/DeploymentSettingsProvider"; -import { useSearchParamsKey } from "hooks/useSearchParamsKey"; export const NotificationsPage: FC = () => { const { deploymentConfig } = useDeploymentSettings(); diff --git a/site/src/pages/DeploymentSettingsPage/ObservabilitySettingsPage/ObservabilitySettingsPage.tsx b/site/src/pages/DeploymentSettingsPage/ObservabilitySettingsPage/ObservabilitySettingsPage.tsx index f116e241399d9..12b574c177384 100644 --- a/site/src/pages/DeploymentSettingsPage/ObservabilitySettingsPage/ObservabilitySettingsPage.tsx +++ b/site/src/pages/DeploymentSettingsPage/ObservabilitySettingsPage/ObservabilitySettingsPage.tsx @@ -1,10 +1,10 @@ import { useDashboard } from "modules/dashboard/useDashboard"; import { useFeatureVisibility } from "modules/dashboard/useFeatureVisibility"; +import { useDeploymentSettings } from "modules/management/DeploymentSettingsProvider"; import type { FC } from "react"; import { Helmet } from "react-helmet-async"; import { pageTitle } from "utils/page"; import { ObservabilitySettingsPageView } from "./ObservabilitySettingsPageView"; -import { useDeploymentSettings } from "modules/management/DeploymentSettingsProvider"; const ObservabilitySettingsPage: FC = () => { const { deploymentConfig } = useDeploymentSettings(); diff --git a/site/src/pages/DeploymentSettingsPage/SecuritySettingsPage/SecuritySettingsPage.tsx b/site/src/pages/DeploymentSettingsPage/SecuritySettingsPage/SecuritySettingsPage.tsx index 4317af0293e5d..bda0988f01966 100644 --- a/site/src/pages/DeploymentSettingsPage/SecuritySettingsPage/SecuritySettingsPage.tsx +++ b/site/src/pages/DeploymentSettingsPage/SecuritySettingsPage/SecuritySettingsPage.tsx @@ -1,10 +1,10 @@ import { Loader } from "components/Loader/Loader"; import { useDashboard } from "modules/dashboard/useDashboard"; +import { useDeploymentSettings } from "modules/management/DeploymentSettingsProvider"; import type { FC } from "react"; import { Helmet } from "react-helmet-async"; import { pageTitle } from "utils/page"; import { SecuritySettingsPageView } from "./SecuritySettingsPageView"; -import { useDeploymentSettings } from "modules/management/DeploymentSettingsProvider"; const SecuritySettingsPage: FC = () => { const { deploymentConfig } = useDeploymentSettings(); diff --git a/site/src/pages/DeploymentSettingsPage/UserAuthSettingsPage/UserAuthSettingsPage.tsx b/site/src/pages/DeploymentSettingsPage/UserAuthSettingsPage/UserAuthSettingsPage.tsx index 7a1f5ca5961ce..1511e29aca2d0 100644 --- a/site/src/pages/DeploymentSettingsPage/UserAuthSettingsPage/UserAuthSettingsPage.tsx +++ b/site/src/pages/DeploymentSettingsPage/UserAuthSettingsPage/UserAuthSettingsPage.tsx @@ -1,9 +1,9 @@ import { Loader } from "components/Loader/Loader"; +import { useDeploymentSettings } from "modules/management/DeploymentSettingsProvider"; import type { FC } from "react"; import { Helmet } from "react-helmet-async"; import { pageTitle } from "utils/page"; import { UserAuthSettingsPageView } from "./UserAuthSettingsPageView"; -import { useDeploymentSettings } from "modules/management/DeploymentSettingsProvider"; const UserAuthSettingsPage: FC = () => { const { deploymentConfig } = useDeploymentSettings(); diff --git a/site/src/testHelpers/storybook.tsx b/site/src/testHelpers/storybook.tsx index 78a97ce163ed5..e905a9b412c2c 100644 --- a/site/src/testHelpers/storybook.tsx +++ b/site/src/testHelpers/storybook.tsx @@ -7,6 +7,7 @@ import { GlobalSnackbar } from "components/GlobalSnackbar/GlobalSnackbar"; import { AuthProvider } from "contexts/auth/AuthProvider"; import { permissionsToCheck } from "contexts/auth/permissions"; import { DashboardContext } from "modules/dashboard/DashboardProvider"; +import { DeploymentSettingsContext } from "modules/management/DeploymentSettingsProvider"; import { ManagementSettingsContext } from "modules/management/ManagementSettingsLayout"; import type { FC } from "react"; import { useQueryClient } from "react-query"; @@ -16,7 +17,6 @@ import { MockDeploymentConfig, MockEntitlements, } from "./entities"; -import { DeploymentSettingsContext } from "modules/management/DeploymentSettingsProvider"; export const withDashboardProvider = ( Story: FC, From 804255e2201333658f4170cf7518aba2624cc2c5 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Mon, 28 Oct 2024 22:32:40 +0000 Subject: [PATCH 26/30] fix: delete invalid tests --- .../ManagementSettingsLayout.test.ts | 188 ------------------ 1 file changed, 188 deletions(-) delete mode 100644 site/src/modules/management/ManagementSettingsLayout.test.ts diff --git a/site/src/modules/management/ManagementSettingsLayout.test.ts b/site/src/modules/management/ManagementSettingsLayout.test.ts deleted file mode 100644 index 2dec371550ad2..0000000000000 --- a/site/src/modules/management/ManagementSettingsLayout.test.ts +++ /dev/null @@ -1,188 +0,0 @@ -import { Permission } from "api/typesGenerated"; -import type { Permissions } from "contexts/auth/permissions"; -import { MockNoPermissions, MockPermissions } from "testHelpers/entities"; -import { isManagementRoutePermitted } from "./ManagementSettingsLayout"; - -describe(isManagementRoutePermitted.name, () => { - describe("General behavior", () => { - it("Rejects malformed routes", () => { - const invalidRoutes: readonly string[] = [ - // It's expected that the hostname will be stripped off - "https://dev.coder.com/deployment/licenses/add", - - // Missing the leading / - "organizations", - ]; - - for (const r of invalidRoutes) { - } - - // Currently only checking whether route ends with / - const invalidRoute = "organizations"; - expect( - isManagementRoutePermitted(invalidRoute, MockPermissions, true), - ).toBe(false); - }); - }); - - describe("Organization routes", () => { - it("Delegates to showOrganizations value for any /organizations routes", () => { - expect( - isManagementRoutePermitted("/organizations", MockNoPermissions, true), - ).toBe(true); - - expect( - isManagementRoutePermitted( - "/organizations/nested/arbitrarily", - MockNoPermissions, - true, - ), - ).toBe(true); - - expect( - isManagementRoutePermitted( - "/organizations/sample-organization", - MockNoPermissions, - true, - ), - ).toBe(true); - - expect( - isManagementRoutePermitted("/organizations", MockPermissions, false), - ).toBe(false); - }); - }); - - describe("Deployment routes", () => { - it("Will never let the user through if they have no active permissions", () => { - let result = isManagementRoutePermitted( - "/deployment", - MockNoPermissions, - true, - ); - expect(result).toBe(false); - - result = isManagementRoutePermitted( - "/deployment/licenses", - MockNoPermissions, - true, - ); - expect(result).toBe(false); - - result = isManagementRoutePermitted( - "/deployment/appearance", - MockNoPermissions, - false, - ); - expect(result).toBe(false); - }); - - it("Will let users access base /deployment route if they have at least one permission", () => { - const mocks: readonly Permissions[] = [ - MockPermissions, - { - ...MockNoPermissions, - createGroup: true, - }, - { - ...MockNoPermissions, - createOrganization: true, - deleteTemplates: true, - }, - { - ...MockNoPermissions, - editDeploymentValues: true, - viewNotificationTemplate: true, - readWorkspaceProxies: true, - }, - ]; - - for (const m of mocks) { - expect(isManagementRoutePermitted("/deployment", m, true)).toBe(true); - expect(isManagementRoutePermitted("/deployment", m, false)).toBe(true); - } - }); - - it("Rejects unknown deployment routes", () => { - const sampleRoutes: readonly string[] = [ - "/deployment/definitely-not-right", - "/deployment/what-is-this", - ]; - - for (const r of sampleRoutes) { - const result = isManagementRoutePermitted(r, MockPermissions, true); - expect(result).toBe(false); - } - }); - - it("Supports deployment routes that are nested more than one level", () => { - const routes: readonly string[] = [ - "/deployment/licenses/add", - - // Including oauth2-provider routes, even though they're not - // currently exposed via the UI - "/deployment/oauth2-provider/apps", - "/deployment/oauth2-provider/apps/add", - ]; - - for (const r of routes) { - let result = isManagementRoutePermitted(r, MockPermissions, true); - expect(result).toBe(true); - - result = isManagementRoutePermitted(r, MockPermissions, false); - expect(result).toBe(true); - } - }); - - it("Granularly associates individual deployment routes with specific permissions", () => { - type PairTuple = readonly [ - route: `/${string}`, - permissionKey: keyof Permissions, - ]; - - const pairs: readonly PairTuple[] = [ - ["/general", "viewDeploymentValues"], - ["/licenses", "viewAllLicenses"], - ["/appearance", "editDeploymentValues"], - ["/userauth", "viewDeploymentValues"], - ["/external-auth", "viewDeploymentValues"], - ["/network", "viewDeploymentValues"], - ["/workspace-proxies", "readWorkspaceProxies"], - ["/security", "viewDeploymentValues"], - ["/observability", "viewDeploymentValues"], - ["/users", "viewAllUsers"], - ["/notifications", "viewNotificationTemplate"], - ["/oauth2-provider", "viewExternalAuthConfig"], - ]; - - for (const [route, permName] of pairs) { - // Using NoPermissions version as baseline to make sure that we - // don't get false positives from all permissions being set to - // true at the start - const mutablePermsCopy = { ...MockNoPermissions }; - const fullRoute = `/deployment${route}`; - - mutablePermsCopy[permName] = true; - let result = isManagementRoutePermitted( - fullRoute, - mutablePermsCopy, - true, - ); - expect(result).toBe(true); - - result = isManagementRoutePermitted( - `${fullRoute}/example-subpath`, - mutablePermsCopy, - true, - ); - expect(result).toBe(true); - - mutablePermsCopy[permName] = false; - result = isManagementRoutePermitted(fullRoute, mutablePermsCopy, true); - expect(result).toBe(false); - } - - expect.hasAssertions(); - }); - }); -}); From d3411754939c88934b950e231889b30904da3c4d Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Mon, 28 Oct 2024 23:04:21 +0000 Subject: [PATCH 27/30] fix: revert changes not caught in merge --- site/src/@types/storybook.d.ts | 1 - .../src/modules/dashboard/DashboardProvider.tsx | 8 -------- site/src/modules/management/Sidebar.tsx | 12 +++++++++--- .../CustomRolesPage/CreateEditRolePage.tsx | 10 +++++----- .../CustomRolesPage/CustomRolesPage.tsx | 8 ++++---- .../GroupsPage/GroupsPage.tsx | 10 +++++----- .../IdpSyncPage/IdpSyncPage.tsx | 8 +++++--- .../OrganizationMembersPage.tsx | 8 ++++---- .../OrganizationProvisionersPage.tsx | 12 ++++-------- .../OrganizationSettingsPage.stories.tsx | 9 +-------- .../OrganizationSettingsPage.tsx | 12 +----------- .../pages/WorkspacePage/WorkspacePage.test.tsx | 1 - .../WorkspacesPageView.stories.tsx | 15 +++++++-------- site/src/router.tsx | 17 ----------------- site/src/testHelpers/storybook.tsx | 11 +---------- 15 files changed, 46 insertions(+), 96 deletions(-) diff --git a/site/src/@types/storybook.d.ts b/site/src/@types/storybook.d.ts index e572f7b385ecf..82507741d5621 100644 --- a/site/src/@types/storybook.d.ts +++ b/site/src/@types/storybook.d.ts @@ -19,7 +19,6 @@ declare module "@storybook/react" { experiments?: Experiments; showOrganizations?: boolean; organizations?: Organization[]; - activeOrganization?: Organization; queries?: { key: QueryKey; data: unknown; isError?: boolean }[]; webSocket?: WebSocketEvent[]; user?: User; diff --git a/site/src/modules/dashboard/DashboardProvider.tsx b/site/src/modules/dashboard/DashboardProvider.tsx index 8dee68a2c53da..d8fa339deccbb 100644 --- a/site/src/modules/dashboard/DashboardProvider.tsx +++ b/site/src/modules/dashboard/DashboardProvider.tsx @@ -13,7 +13,6 @@ import { Loader } from "components/Loader/Loader"; import { useEmbeddedMetadata } from "hooks/useEmbeddedMetadata"; import { type FC, type PropsWithChildren, createContext } from "react"; import { useQuery } from "react-query"; -import { useParams } from "react-router-dom"; import { selectFeatureVisibility } from "./entitlements"; export interface DashboardValue { @@ -21,7 +20,6 @@ export interface DashboardValue { experiments: Experiments; appearance: AppearanceConfig; organizations: readonly Organization[]; - activeOrganization: Organization | undefined; showOrganizations: boolean; } @@ -35,9 +33,6 @@ export const DashboardProvider: FC = ({ children }) => { const experimentsQuery = useQuery(experiments(metadata.experiments)); const appearanceQuery = useQuery(appearance(metadata.appearance)); const organizationsQuery = useQuery(organizations()); - const { organization: organizationName } = useParams() as { - organization?: string; - }; const error = entitlementsQuery.error || @@ -72,9 +67,6 @@ export const DashboardProvider: FC = ({ children }) => { appearance: appearanceQuery.data, organizations: organizationsQuery.data, showOrganizations: hasMultipleOrganizations || organizationsEnabled, - activeOrganization: organizationsQuery.data?.find( - (org) => org.name === organizationName, - ), }} > {children} diff --git a/site/src/modules/management/Sidebar.tsx b/site/src/modules/management/Sidebar.tsx index 597ca1c5ec3ad..a2560fe5d6515 100644 --- a/site/src/modules/management/Sidebar.tsx +++ b/site/src/modules/management/Sidebar.tsx @@ -1,7 +1,10 @@ import { organizationsPermissions } from "api/queries/organizations"; import { useAuthenticated } from "contexts/auth/RequireAuth"; import { useDashboard } from "modules/dashboard/useDashboard"; -import { canEditOrganization } from "modules/management/ManagementSettingsLayout"; +import { + canEditOrganization, + useManagementSettings, +} from "modules/management/ManagementSettingsLayout"; import type { FC } from "react"; import { useQuery } from "react-query"; import { useLocation, useParams } from "react-router-dom"; @@ -17,7 +20,10 @@ import { type OrganizationWithPermissions, SidebarView } from "./SidebarView"; export const Sidebar: FC = () => { const location = useLocation(); const { permissions } = useAuthenticated(); - const { organizations, activeOrganization } = useDashboard(); + const { organizations } = useManagementSettings(); + const { organization: organizationName } = useParams() as { + organization?: string; + }; const orgPermissionsQuery = useQuery( organizationsPermissions(organizations?.map((o) => o.id)), @@ -46,7 +52,7 @@ export const Sidebar: FC = () => { // the user is on /organizations but has no editable organizations to // which we can redirect. activeSettings={location.pathname.startsWith("/deployment")} - activeOrganizationName={activeOrganization?.name} + activeOrganizationName={organizationName} organizations={editableOrgs} permissions={permissions} /> diff --git a/site/src/pages/ManagementSettingsPage/CustomRolesPage/CreateEditRolePage.tsx b/site/src/pages/ManagementSettingsPage/CustomRolesPage/CreateEditRolePage.tsx index 6701a52e68f63..581b64d2eefd8 100644 --- a/site/src/pages/ManagementSettingsPage/CustomRolesPage/CreateEditRolePage.tsx +++ b/site/src/pages/ManagementSettingsPage/CustomRolesPage/CreateEditRolePage.tsx @@ -8,7 +8,7 @@ import { import type { CustomRoleRequest } from "api/typesGenerated"; import { displayError } from "components/GlobalSnackbar/utils"; import { Loader } from "components/Loader/Loader"; -import { useDashboard } from "modules/dashboard/useDashboard"; +import { useManagementSettings } from "modules/management/ManagementSettingsLayout"; import type { FC } from "react"; import { Helmet } from "react-helmet-async"; import { useMutation, useQuery, useQueryClient } from "react-query"; @@ -19,15 +19,15 @@ import CreateEditRolePageView from "./CreateEditRolePageView"; export const CreateEditRolePage: FC = () => { const queryClient = useQueryClient(); const navigate = useNavigate(); - const { activeOrganization } = useDashboard(); + + const { organizations } = useManagementSettings(); const { organization: organizationName, roleName } = useParams() as { organization: string; roleName: string; }; + const organization = organizations?.find((o) => o.name === organizationName); + const permissionsQuery = useQuery(organizationPermissions(organization?.id)); - const permissionsQuery = useQuery( - organizationPermissions(activeOrganization?.id), - ); const createOrganizationRoleMutation = useMutation( createOrganizationRole(queryClient, organizationName), ); diff --git a/site/src/pages/ManagementSettingsPage/CustomRolesPage/CustomRolesPage.tsx b/site/src/pages/ManagementSettingsPage/CustomRolesPage/CustomRolesPage.tsx index d6b225629051d..d55f882beb2d2 100644 --- a/site/src/pages/ManagementSettingsPage/CustomRolesPage/CustomRolesPage.tsx +++ b/site/src/pages/ManagementSettingsPage/CustomRolesPage/CustomRolesPage.tsx @@ -9,6 +9,7 @@ import { SettingsHeader } from "components/SettingsHeader/SettingsHeader"; import { Stack } from "components/Stack/Stack"; import { useDashboard } from "modules/dashboard/useDashboard"; import { useFeatureVisibility } from "modules/dashboard/useFeatureVisibility"; +import { useManagementSettings } from "modules/management/ManagementSettingsLayout"; import { type FC, useEffect, useState } from "react"; import { Helmet } from "react-helmet-async"; import { useMutation, useQuery, useQueryClient } from "react-query"; @@ -22,10 +23,9 @@ export const CustomRolesPage: FC = () => { const { organization: organizationName } = useParams() as { organization: string; }; - const { activeOrganization } = useDashboard(); - const permissionsQuery = useQuery( - organizationPermissions(activeOrganization?.id), - ); + const { organizations } = useManagementSettings(); + const organization = organizations?.find((o) => o.name === organizationName); + const permissionsQuery = useQuery(organizationPermissions(organization?.id)); const deleteRoleMutation = useMutation( deleteOrganizationRole(queryClient, organizationName), ); diff --git a/site/src/pages/ManagementSettingsPage/GroupsPage/GroupsPage.tsx b/site/src/pages/ManagementSettingsPage/GroupsPage/GroupsPage.tsx index 1524ac944cce7..4947045a0f70d 100644 --- a/site/src/pages/ManagementSettingsPage/GroupsPage/GroupsPage.tsx +++ b/site/src/pages/ManagementSettingsPage/GroupsPage/GroupsPage.tsx @@ -11,6 +11,7 @@ import { SettingsHeader } from "components/SettingsHeader/SettingsHeader"; import { Stack } from "components/Stack/Stack"; import { useDashboard } from "modules/dashboard/useDashboard"; import { useFeatureVisibility } from "modules/dashboard/useFeatureVisibility"; +import { useManagementSettings } from "modules/management/ManagementSettingsLayout"; import { type FC, useEffect } from "react"; import { Helmet } from "react-helmet-async"; import { useQuery } from "react-query"; @@ -24,10 +25,9 @@ export const GroupsPage: FC = () => { organization: string; }; const groupsQuery = useQuery(groupsByOrganization(organizationName)); - const { organizations, activeOrganization } = useDashboard(); - const permissionsQuery = useQuery( - organizationPermissions(activeOrganization?.id), - ); + const { organizations } = useManagementSettings(); + const organization = organizations?.find((o) => o.name === organizationName); + const permissionsQuery = useQuery(organizationPermissions(organization?.id)); useEffect(() => { if (groupsQuery.error) { @@ -58,7 +58,7 @@ export const GroupsPage: FC = () => { throw new Error("No default organization found"); } - if (!activeOrganization) { + if (!organization) { return ; } diff --git a/site/src/pages/ManagementSettingsPage/IdpSyncPage/IdpSyncPage.tsx b/site/src/pages/ManagementSettingsPage/IdpSyncPage/IdpSyncPage.tsx index cccc4eb9d386a..a2969890beefb 100644 --- a/site/src/pages/ManagementSettingsPage/IdpSyncPage/IdpSyncPage.tsx +++ b/site/src/pages/ManagementSettingsPage/IdpSyncPage/IdpSyncPage.tsx @@ -12,6 +12,7 @@ import { SettingsHeader } from "components/SettingsHeader/SettingsHeader"; import { Stack } from "components/Stack/Stack"; import { useDashboard } from "modules/dashboard/useDashboard"; import { useFeatureVisibility } from "modules/dashboard/useFeatureVisibility"; +import { useManagementSettings } from "modules/management/ManagementSettingsLayout"; import type { FC } from "react"; import { Helmet } from "react-helmet-async"; import { useQueries } from "react-query"; @@ -27,7 +28,8 @@ export const IdpSyncPage: FC = () => { }; // IdP sync does not have its own entitlement and is based on templace_rbac const { template_rbac: isIdpSyncEnabled } = useFeatureVisibility(); - const { activeOrganization } = useDashboard(); + const { organizations } = useManagementSettings(); + const organization = organizations?.find((o) => o.name === organizationName); const [groupIdpSyncSettingsQuery, roleIdpSyncSettingsQuery, groupsQuery] = useQueries({ @@ -38,7 +40,7 @@ export const IdpSyncPage: FC = () => { ], }); - if (!activeOrganization) { + if (!organization) { return ; } @@ -93,7 +95,7 @@ export const IdpSyncPage: FC = () => { roleSyncSettings={roleIdpSyncSettingsQuery.data} groups={groupsQuery.data} groupsMap={groupsMap} - organization={activeOrganization} + organization={organization} error={error} /> diff --git a/site/src/pages/ManagementSettingsPage/OrganizationMembersPage.tsx b/site/src/pages/ManagementSettingsPage/OrganizationMembersPage.tsx index c0de1ef1bc16d..be194b880f822 100644 --- a/site/src/pages/ManagementSettingsPage/OrganizationMembersPage.tsx +++ b/site/src/pages/ManagementSettingsPage/OrganizationMembersPage.tsx @@ -16,6 +16,7 @@ import { Loader } from "components/Loader/Loader"; import { Stack } from "components/Stack/Stack"; import { useAuthenticated } from "contexts/auth/RequireAuth"; import { useDashboard } from "modules/dashboard/useDashboard"; +import { useManagementSettings } from "modules/management/ManagementSettingsLayout"; import { type FC, useState } from "react"; import { useMutation, useQuery, useQueryClient } from "react-query"; import { useParams } from "react-router-dom"; @@ -50,10 +51,9 @@ const OrganizationMembersPage: FC = () => { updateOrganizationMemberRoles(queryClient, organizationName), ); - const { activeOrganization } = useDashboard(); - const permissionsQuery = useQuery( - organizationPermissions(activeOrganization?.id), - ); + const { organizations } = useManagementSettings(); + const organization = organizations?.find((o) => o.name === organizationName); + const permissionsQuery = useQuery(organizationPermissions(organization?.id)); const [memberToDelete, setMemberToDelete] = useState(); diff --git a/site/src/pages/ManagementSettingsPage/OrganizationProvisionersPage.tsx b/site/src/pages/ManagementSettingsPage/OrganizationProvisionersPage.tsx index 62c5a60f01e9f..c58e0bf7b1942 100644 --- a/site/src/pages/ManagementSettingsPage/OrganizationProvisionersPage.tsx +++ b/site/src/pages/ManagementSettingsPage/OrganizationProvisionersPage.tsx @@ -4,6 +4,7 @@ import type { Organization } from "api/typesGenerated"; import { EmptyState } from "components/EmptyState/EmptyState"; import { useEmbeddedMetadata } from "hooks/useEmbeddedMetadata"; import { useDashboard } from "modules/dashboard/useDashboard"; +import { useManagementSettings } from "modules/management/ManagementSettingsLayout"; import type { FC } from "react"; import { useQuery } from "react-query"; import { useParams } from "react-router-dom"; @@ -13,14 +14,14 @@ const OrganizationProvisionersPage: FC = () => { const { organization: organizationName } = useParams() as { organization: string; }; - const { activeOrganization } = useDashboard(); const { entitlements } = useDashboard(); - + const { organizations } = useManagementSettings(); const { metadata } = useEmbeddedMetadata(); const buildInfoQuery = useQuery(buildInfo(metadata["build-info"])); const provisionersQuery = useQuery(provisionerDaemonGroups(organizationName)); - if (!activeOrganization) { + const organization = organizations?.find((o) => o.name === organizationName); + if (!organization) { return ; } @@ -35,8 +36,3 @@ const OrganizationProvisionersPage: FC = () => { }; export default OrganizationProvisionersPage; - -const getOrganizationByName = ( - organizations: readonly Organization[], - name: string, -) => organizations.find((org) => org.name === name); diff --git a/site/src/pages/ManagementSettingsPage/OrganizationSettingsPage.stories.tsx b/site/src/pages/ManagementSettingsPage/OrganizationSettingsPage.stories.tsx index acdb088a64c04..f6b6b49c88d37 100644 --- a/site/src/pages/ManagementSettingsPage/OrganizationSettingsPage.stories.tsx +++ b/site/src/pages/ManagementSettingsPage/OrganizationSettingsPage.stories.tsx @@ -38,17 +38,10 @@ const meta: Meta = { export default meta; type Story = StoryObj; -export const NoRedirectableOrganizations: Story = { - parameters: { - organizations: [MockDefaultOrganization], - activeOrganization: undefined, - }, -}; +export const NoRedirectableOrganizations: Story = {}; export const OrganizationDoesNotExist: Story = { parameters: { - organizations: [MockDefaultOrganization], - activeOrganization: MockOrganization2, reactRouter: reactRouterParameters({ location: { pathParams: { organization: "does-not-exist" } }, routing: { path: "/organizations/:organization" }, diff --git a/site/src/pages/ManagementSettingsPage/OrganizationSettingsPage.tsx b/site/src/pages/ManagementSettingsPage/OrganizationSettingsPage.tsx index 63dd6a2d47ce4..909c8bc5cb443 100644 --- a/site/src/pages/ManagementSettingsPage/OrganizationSettingsPage.tsx +++ b/site/src/pages/ManagementSettingsPage/OrganizationSettingsPage.tsx @@ -35,10 +35,7 @@ const OrganizationSettingsPage: FC = () => { deleteOrganization(queryClient), ); - const organization = - organizations && organizationName - ? getOrganizationByName(organizations, organizationName) - : undefined; + const organization = organizations?.find((o) => o.name === organizationName); const permissionsQuery = useQuery( organizationsPermissions(organizations?.map((o) => o.id)), ); @@ -116,10 +113,3 @@ const OrganizationSettingsPage: FC = () => { }; export default OrganizationSettingsPage; - -const getOrganizationByName = ( - organizations: readonly Organization[], - name: string, -) => { - return organizations.find((org) => org.name === name); -}; diff --git a/site/src/pages/WorkspacePage/WorkspacePage.test.tsx b/site/src/pages/WorkspacePage/WorkspacePage.test.tsx index 0ec52f7bd3768..2c113b933483e 100644 --- a/site/src/pages/WorkspacePage/WorkspacePage.test.tsx +++ b/site/src/pages/WorkspacePage/WorkspacePage.test.tsx @@ -562,7 +562,6 @@ describe("WorkspacePage", () => { entitlements: MockEntitlements, experiments: [], organizations: [MockOrganization], - activeOrganization: MockOrganization, showOrganizations: true, }} > diff --git a/site/src/pages/WorkspacesPage/WorkspacesPageView.stories.tsx b/site/src/pages/WorkspacesPage/WorkspacesPageView.stories.tsx index c74a1f708cc28..4460038112cd1 100644 --- a/site/src/pages/WorkspacesPage/WorkspacesPageView.stories.tsx +++ b/site/src/pages/WorkspacesPage/WorkspacesPageView.stories.tsx @@ -270,12 +270,6 @@ export const InvalidPageNumber: Story = { }, }; -const mockOrganization: Organization = { - ...MockOrganization, - name: "limbus-co", - display_name: "Limbus Company, LLC", -}; - export const ShowOrganizations: Story = { args: { workspaces: [{ ...MockWorkspace, organization_name: "limbus-co" }], @@ -283,8 +277,13 @@ export const ShowOrganizations: Story = { parameters: { showOrganizations: true, - activeOrganization: mockOrganization, - organizations: [mockOrganization], + organizations: [ + { + ...MockOrganization, + name: "limbus-co", + display_name: "Limbus Company, LLC", + }, + ], }, play: async ({ canvasElement }) => { diff --git a/site/src/router.tsx b/site/src/router.tsx index 41ffee2d2780b..c9d8736979c34 100644 --- a/site/src/router.tsx +++ b/site/src/router.tsx @@ -431,23 +431,6 @@ export const router = createBrowserRouter( - {/** - * @todo 2024-10-24 - MES - The current deployment page is - * almost empty, which isn't great for two reasons: - * - * 1. Even though none of our links lead to the page, the - * user is still able to navigate straight there by - * accident. - * 2. If the user has access to some deployment pages, but - * tries accessing one that they shouldn't be able to - * access, we have to reroute them somewhere. The base - * deployment route is the only safe option, because not - * even the general page is accessible by everyone. - * - * Should update the base /deployment page so that there's - * something there, but that will require coordination with - * the design team. - */} }> }> } /> diff --git a/site/src/testHelpers/storybook.tsx b/site/src/testHelpers/storybook.tsx index 75b97cde237ba..e905a9b412c2c 100644 --- a/site/src/testHelpers/storybook.tsx +++ b/site/src/testHelpers/storybook.tsx @@ -26,17 +26,9 @@ export const withDashboardProvider = ( features = [], experiments = [], showOrganizations = false, + organizations = [MockDefaultOrganization], } = parameters; - // Only set a default value for activeOrganizations if the original - // organizations array wasn't specified. In some cases, we want to have a - // list of organizations, but not have one be active - let { organizations, activeOrganization } = parameters; - if (organizations === undefined) { - organizations = [MockDefaultOrganization]; - activeOrganization = MockDefaultOrganization; - } - const entitlements: Entitlements = { ...MockEntitlements, has_license: features.length > 0, @@ -57,7 +49,6 @@ export const withDashboardProvider = ( experiments, organizations, showOrganizations, - activeOrganization, appearance: MockAppearanceConfig, }} > From f3e66591a7bf0116cd3edb6ed50c4623aff414cf Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Mon, 28 Oct 2024 23:07:41 +0000 Subject: [PATCH 28/30] fix: update spellcheck --- .vscode/settings.json | 1 - 1 file changed, 1 deletion(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index cc4986706f7f8..6695a12faa8dc 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -174,7 +174,6 @@ "typesafe", "unauthenticate", "unconvert", - "unpermitted", "untar", "userauth", "userspace", From ce67a6631e3af4132dbb8c573cd455d64f64304c Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Mon, 28 Oct 2024 23:16:41 +0000 Subject: [PATCH 29/30] fix: revert nits --- .../CustomRolesPage/CreateEditRolePage.tsx | 3 +-- .../CustomRolesPage/CustomRolesPage.tsx | 1 - .../pages/ManagementSettingsPage/GroupsPage/GroupsPage.tsx | 1 - .../pages/ManagementSettingsPage/IdpSyncPage/IdpSyncPage.tsx | 1 - .../pages/ManagementSettingsPage/OrganizationMembersPage.tsx | 1 - .../ManagementSettingsPage/OrganizationProvisionersPage.tsx | 5 +++-- site/src/pages/WorkspacesPage/WorkspacesPageView.stories.tsx | 1 - 7 files changed, 4 insertions(+), 9 deletions(-) diff --git a/site/src/pages/ManagementSettingsPage/CustomRolesPage/CreateEditRolePage.tsx b/site/src/pages/ManagementSettingsPage/CustomRolesPage/CreateEditRolePage.tsx index 581b64d2eefd8..e770a400af2a7 100644 --- a/site/src/pages/ManagementSettingsPage/CustomRolesPage/CreateEditRolePage.tsx +++ b/site/src/pages/ManagementSettingsPage/CustomRolesPage/CreateEditRolePage.tsx @@ -20,14 +20,13 @@ export const CreateEditRolePage: FC = () => { const queryClient = useQueryClient(); const navigate = useNavigate(); - const { organizations } = useManagementSettings(); const { organization: organizationName, roleName } = useParams() as { organization: string; roleName: string; }; + const { organizations } = useManagementSettings(); const organization = organizations?.find((o) => o.name === organizationName); const permissionsQuery = useQuery(organizationPermissions(organization?.id)); - const createOrganizationRoleMutation = useMutation( createOrganizationRole(queryClient, organizationName), ); diff --git a/site/src/pages/ManagementSettingsPage/CustomRolesPage/CustomRolesPage.tsx b/site/src/pages/ManagementSettingsPage/CustomRolesPage/CustomRolesPage.tsx index d55f882beb2d2..34c33083a76be 100644 --- a/site/src/pages/ManagementSettingsPage/CustomRolesPage/CustomRolesPage.tsx +++ b/site/src/pages/ManagementSettingsPage/CustomRolesPage/CustomRolesPage.tsx @@ -7,7 +7,6 @@ import { displayError, displaySuccess } from "components/GlobalSnackbar/utils"; import { Loader } from "components/Loader/Loader"; import { SettingsHeader } from "components/SettingsHeader/SettingsHeader"; import { Stack } from "components/Stack/Stack"; -import { useDashboard } from "modules/dashboard/useDashboard"; import { useFeatureVisibility } from "modules/dashboard/useFeatureVisibility"; import { useManagementSettings } from "modules/management/ManagementSettingsLayout"; import { type FC, useEffect, useState } from "react"; diff --git a/site/src/pages/ManagementSettingsPage/GroupsPage/GroupsPage.tsx b/site/src/pages/ManagementSettingsPage/GroupsPage/GroupsPage.tsx index 4947045a0f70d..774360dc6a6d1 100644 --- a/site/src/pages/ManagementSettingsPage/GroupsPage/GroupsPage.tsx +++ b/site/src/pages/ManagementSettingsPage/GroupsPage/GroupsPage.tsx @@ -9,7 +9,6 @@ import { displayError } from "components/GlobalSnackbar/utils"; import { Loader } from "components/Loader/Loader"; import { SettingsHeader } from "components/SettingsHeader/SettingsHeader"; import { Stack } from "components/Stack/Stack"; -import { useDashboard } from "modules/dashboard/useDashboard"; import { useFeatureVisibility } from "modules/dashboard/useFeatureVisibility"; import { useManagementSettings } from "modules/management/ManagementSettingsLayout"; import { type FC, useEffect } from "react"; diff --git a/site/src/pages/ManagementSettingsPage/IdpSyncPage/IdpSyncPage.tsx b/site/src/pages/ManagementSettingsPage/IdpSyncPage/IdpSyncPage.tsx index a2969890beefb..ef432e8b0d6d6 100644 --- a/site/src/pages/ManagementSettingsPage/IdpSyncPage/IdpSyncPage.tsx +++ b/site/src/pages/ManagementSettingsPage/IdpSyncPage/IdpSyncPage.tsx @@ -10,7 +10,6 @@ import { EmptyState } from "components/EmptyState/EmptyState"; import { Paywall } from "components/Paywall/Paywall"; import { SettingsHeader } from "components/SettingsHeader/SettingsHeader"; import { Stack } from "components/Stack/Stack"; -import { useDashboard } from "modules/dashboard/useDashboard"; import { useFeatureVisibility } from "modules/dashboard/useFeatureVisibility"; import { useManagementSettings } from "modules/management/ManagementSettingsLayout"; import type { FC } from "react"; diff --git a/site/src/pages/ManagementSettingsPage/OrganizationMembersPage.tsx b/site/src/pages/ManagementSettingsPage/OrganizationMembersPage.tsx index be194b880f822..de6295a838bd9 100644 --- a/site/src/pages/ManagementSettingsPage/OrganizationMembersPage.tsx +++ b/site/src/pages/ManagementSettingsPage/OrganizationMembersPage.tsx @@ -15,7 +15,6 @@ import { displayError, displaySuccess } from "components/GlobalSnackbar/utils"; import { Loader } from "components/Loader/Loader"; import { Stack } from "components/Stack/Stack"; import { useAuthenticated } from "contexts/auth/RequireAuth"; -import { useDashboard } from "modules/dashboard/useDashboard"; import { useManagementSettings } from "modules/management/ManagementSettingsLayout"; import { type FC, useState } from "react"; import { useMutation, useQuery, useQueryClient } from "react-query"; diff --git a/site/src/pages/ManagementSettingsPage/OrganizationProvisionersPage.tsx b/site/src/pages/ManagementSettingsPage/OrganizationProvisionersPage.tsx index c58e0bf7b1942..be56ef081fe53 100644 --- a/site/src/pages/ManagementSettingsPage/OrganizationProvisionersPage.tsx +++ b/site/src/pages/ManagementSettingsPage/OrganizationProvisionersPage.tsx @@ -14,13 +14,14 @@ const OrganizationProvisionersPage: FC = () => { const { organization: organizationName } = useParams() as { organization: string; }; - const { entitlements } = useDashboard(); const { organizations } = useManagementSettings(); + const { entitlements } = useDashboard(); const { metadata } = useEmbeddedMetadata(); const buildInfoQuery = useQuery(buildInfo(metadata["build-info"])); - const provisionersQuery = useQuery(provisionerDaemonGroups(organizationName)); const organization = organizations?.find((o) => o.name === organizationName); + const provisionersQuery = useQuery(provisionerDaemonGroups(organizationName)); + if (!organization) { return ; } diff --git a/site/src/pages/WorkspacesPage/WorkspacesPageView.stories.tsx b/site/src/pages/WorkspacesPage/WorkspacesPageView.stories.tsx index 4460038112cd1..ef639d087fb5a 100644 --- a/site/src/pages/WorkspacesPage/WorkspacesPageView.stories.tsx +++ b/site/src/pages/WorkspacesPage/WorkspacesPageView.stories.tsx @@ -1,7 +1,6 @@ import type { Meta, StoryObj } from "@storybook/react"; import { expect, within } from "@storybook/test"; import { - type Organization, type Workspace, type WorkspaceStatus, WorkspaceStatuses, From 7e946a7be2fe34f971488916013f5850d1c33b10 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Tue, 29 Oct 2024 04:56:30 +0000 Subject: [PATCH 30/30] fix: apply feedback --- .../OrganizationProvisionersPage.tsx | 4 +--- .../OrganizationSettingsPage.tsx | 10 +--------- 2 files changed, 2 insertions(+), 12 deletions(-) diff --git a/site/src/pages/ManagementSettingsPage/OrganizationProvisionersPage.tsx b/site/src/pages/ManagementSettingsPage/OrganizationProvisionersPage.tsx index be56ef081fe53..19387a28730eb 100644 --- a/site/src/pages/ManagementSettingsPage/OrganizationProvisionersPage.tsx +++ b/site/src/pages/ManagementSettingsPage/OrganizationProvisionersPage.tsx @@ -14,12 +14,10 @@ const OrganizationProvisionersPage: FC = () => { const { organization: organizationName } = useParams() as { organization: string; }; - const { organizations } = useManagementSettings(); + const { organization } = useManagementSettings(); const { entitlements } = useDashboard(); const { metadata } = useEmbeddedMetadata(); const buildInfoQuery = useQuery(buildInfo(metadata["build-info"])); - - const organization = organizations?.find((o) => o.name === organizationName); const provisionersQuery = useQuery(provisionerDaemonGroups(organizationName)); if (!organization) { diff --git a/site/src/pages/ManagementSettingsPage/OrganizationSettingsPage.tsx b/site/src/pages/ManagementSettingsPage/OrganizationSettingsPage.tsx index 909c8bc5cb443..2b4eb18a9a524 100644 --- a/site/src/pages/ManagementSettingsPage/OrganizationSettingsPage.tsx +++ b/site/src/pages/ManagementSettingsPage/OrganizationSettingsPage.tsx @@ -55,15 +55,7 @@ const OrganizationSettingsPage: FC = () => { // .find will stop at the first match found; make sure default // organizations are placed first const editableOrg = [...organizations] - .sort((a, b) => { - if (a.is_default && !b.is_default) { - return -1; - } - if (b.is_default && !a.is_default) { - return 1; - } - return 0; - }) + .sort((a, b) => (b.is_default ? 1 : 0) - (a.is_default ? 1 : 0)) .find((org) => canEditOrganization(permissions[org.id])); if (editableOrg) { return ;