diff --git a/app/components/ErrorBoundary.tsx b/app/components/ErrorBoundary.tsx index cbbcb923bd..347e1f1265 100644 --- a/app/components/ErrorBoundary.tsx +++ b/app/components/ErrorBoundary.tsx @@ -5,6 +5,8 @@ import type { ErrorResult } from '@oxide/api' import NotFound from 'app/pages/NotFound' +export const trigger404 = { type: 'error', statusCode: 404 } + type Props = { error: Error | ErrorResult } function ErrorFallback({ error }: Props) { diff --git a/app/components/TopBar.tsx b/app/components/TopBar.tsx index 64cb3e8080..291578c764 100644 --- a/app/components/TopBar.tsx +++ b/app/components/TopBar.tsx @@ -1,5 +1,6 @@ import { Menu, MenuButton, MenuItem, MenuList } from '@reach/menu-button' -import { useLocation, useNavigate, useParams } from 'react-router-dom' +import React from 'react' +import { useNavigate } from 'react-router-dom' import { navToLogin, useApiMutation, useApiQuery } from '@oxide/api' import { @@ -9,19 +10,10 @@ import { Notifications16Icon, Profile16Icon, } from '@oxide/ui' -import { isTruthy } from '@oxide/util' import { pb } from 'app/util/path-builder' -import { OrgPicker, ProjectPicker, SiloSystemPicker } from './TopBarPicker' - -/** - * TODO: This is a temporary flag to disable the silo picker until we have - * the an API endpoint to support it. - */ -const hasSiloPerms = true - -export function TopBar() { +export function TopBar({ children }: { children: React.ReactNode }) { const navigate = useNavigate() const logout = useApiMutation('logout', { onSuccess: () => { @@ -31,29 +23,13 @@ export function TopBar() { navToLogin({ includeCurrent: false }) }, }) - const { data: user, error } = useApiQuery( - 'sessionMe', - {}, - { cacheTime: 0, refetchOnWindowFocus: false } - ) - - const loggedIn = user && !error + const { data: user } = useApiQuery('sessionMe', {}, { cacheTime: 0 }) - const isSystem = useLocation().pathname.startsWith(pb.system()) // lol - const { projectName } = useParams() + const loggedIn = !!user - const [cornerPicker, ...otherPickers] = [ - hasSiloPerms && , - // TODO: This works ok in most situations, but when an operator user is on - // the orgs page with no org selected, they see this picker, which is - // redundant with the list of orgs. Overall this logic is starting to feel - // silly, which points to a non-centralized approach handled in the layouts - // like we were doing before. That way, for example, we know whether we're - // on a system situation because we're in SystemLayout. Seems pretty obvious - // in hindsight. - !isSystem && , - projectName && , - ].filter(isTruthy) + // toArray filters out nulls, which is essential because the silo/system + // picker is going to come in null when the user isn't supposed to see it + const [cornerPicker, ...otherPickers] = React.Children.toArray(children) // The height of this component is governed by the `PageContainer` // It's important that this component returns two distinct elements (wrapped in a fragment). diff --git a/app/components/TopBarPicker.tsx b/app/components/TopBarPicker.tsx index 2e05a16cfe..6b0b81215d 100644 --- a/app/components/TopBarPicker.tsx +++ b/app/components/TopBarPicker.tsx @@ -104,7 +104,29 @@ const NoOrgLogo = () => ( ) -export function SiloSystemPicker({ isSystem }: { isSystem: boolean }) { +/** + * Return `null` instead of the picker when the user doesn't have fleet viewer + * perms so it can get filtered out of the children array in ``. Having + * `` itself return `null` from render doesn't work because + * `` can't see that. + */ +export function useSiloSystemPicker(value: 'silo' | 'system') { + // User is only shown system routes if they have viewer perms (at least) on + // the fleet. The natural place to find out whether they have such perms is + // the fleet policy, but if the user doesn't have fleet read, we'll get a 403 + // from that endpoint. So we simply check whether that endpoint 200s or not to + // determine whether the user is a fleet viewer. + // + // Note that we are able to ignore the possibility of `systemPolicy` being + // undefined due to the request being in flight because we have prefetched + // this request in the loader. If that prefetch were removed, fleet viewers + // would see the silo picker pop in when the request resolves, which would be + // bad. + const { data: systemPolicy } = useApiQuery('systemPolicyView', {}) + return systemPolicy ? : null +} + +export function SiloSystemPicker({ value }: { value: 'silo' | 'system' }) { const commonProps = { items: [ { label: 'System', to: pb.silos() }, @@ -113,7 +135,7 @@ export function SiloSystemPicker({ isSystem }: { isSystem: boolean }) { 'aria-label': 'Switch between system and silo', } - return isSystem ? ( + return value === 'system' ? ( { return ( - + + {useSiloSystemPicker('silo')} + + diff --git a/app/layouts/ProjectLayout.tsx b/app/layouts/ProjectLayout.tsx index 8b73524dd7..afdb5afc8a 100644 --- a/app/layouts/ProjectLayout.tsx +++ b/app/layouts/ProjectLayout.tsx @@ -13,6 +13,7 @@ import { } from '@oxide/ui' import { TopBar } from 'app/components/TopBar' +import { OrgPicker, ProjectPicker, useSiloSystemPicker } from 'app/components/TopBarPicker' import { useQuickActions, useRequiredParams } from 'app/hooks' import { pb } from 'app/util/path-builder' @@ -49,7 +50,11 @@ const ProjectLayout = () => { return ( - + + {useSiloSystemPicker('silo')} + + + diff --git a/app/layouts/SettingsLayout.tsx b/app/layouts/SettingsLayout.tsx index 6e4d9f73b6..de8ac6f3d3 100644 --- a/app/layouts/SettingsLayout.tsx +++ b/app/layouts/SettingsLayout.tsx @@ -4,6 +4,7 @@ import { matchPath, useLocation, useNavigate } from 'react-router-dom' import { Divider, Key16Icon, Profile16Icon, Show16Icon } from '@oxide/ui' import { TopBar } from 'app/components/TopBar' +import { OrgPicker, useSiloSystemPicker } from 'app/components/TopBarPicker' import { useQuickActions } from 'app/hooks' import { pb } from 'app/util/path-builder' @@ -37,7 +38,10 @@ const SettingsLayout = () => { return ( - + + {useSiloSystemPicker('silo')} + + {/* TODO: what to link here? anything? */} diff --git a/app/layouts/SiloLayout.tsx b/app/layouts/SiloLayout.tsx index 93ba513305..bca1ac8545 100644 --- a/app/layouts/SiloLayout.tsx +++ b/app/layouts/SiloLayout.tsx @@ -2,6 +2,7 @@ import { Divider, Organization16Icon, Snapshots16Icon } from '@oxide/ui' import { DocsLinkItem, NavLinkItem, Sidebar } from 'app/components/Sidebar' import { TopBar } from 'app/components/TopBar' +import { OrgPicker, useSiloSystemPicker } from 'app/components/TopBarPicker' import { pb } from 'app/util/path-builder' import { ContentPane, PageContainer } from './helpers' @@ -9,7 +10,10 @@ import { ContentPane, PageContainer } from './helpers' export default function SiloLayout() { return ( - + + {useSiloSystemPicker('silo')} + + diff --git a/app/layouts/SystemLayout.tsx b/app/layouts/SystemLayout.tsx index 932fa5cd25..0397bd7db2 100644 --- a/app/layouts/SystemLayout.tsx +++ b/app/layouts/SystemLayout.tsx @@ -1,3 +1,4 @@ +import { apiQueryClient } from '@oxide/api' import { Cloud16Icon, Divider, @@ -10,16 +11,40 @@ import { Storage16Icon, } from '@oxide/ui' +import { trigger404 } from 'app/components/ErrorBoundary' import { DocsLinkItem, NavLinkItem, Sidebar } from 'app/components/Sidebar' import { TopBar } from 'app/components/TopBar' +import { SiloSystemPicker } from 'app/components/TopBarPicker' import { pb } from 'app/util/path-builder' import { ContentPane, PageContainer } from './helpers' -export default function SiloLayout() { +/** + * If we can see the policy, we're a fleet viewer, and we need to be a fleet + * viewer in order to see any of the routes under this layout. We need to + * `fetchQuery` instead of `prefetchQuery` because the latter doesn't return the + * result, and then we need to `.catch()` because `fetchQuery` throws on request + * error. We're being a little cavalier here with the error. If it's something + * other than a 403, that would be strange and we would want to know. + */ +SystemLayout.loader = async () => { + const isFleetViewer = await apiQueryClient + .fetchQuery('systemPolicyView', {}) + .then(() => true) + .catch(() => false) + + // TODO: make sure 404 is the desired behavior. This situation should be + // pretty unlikely. + if (!isFleetViewer) throw trigger404 +} + +export default function SystemLayout() { return ( - + + {/* don't use the hook bc if we're here, this needs to show up */} + + diff --git a/app/layouts/helpers.tsx b/app/layouts/helpers.tsx index 64466739cb..e631e06640 100644 --- a/app/layouts/helpers.tsx +++ b/app/layouts/helpers.tsx @@ -24,6 +24,13 @@ export const ContentPane = () => ( ) -export async function prefetchSessionMe() { - await apiQueryClient.prefetchQuery('sessionMe', {}) +/** Loader for the `` that wraps all authenticated routes. */ +export const userLoader = async () => { + await Promise.all([ + apiQueryClient.prefetchQuery('sessionMe', {}), + // Need to prefetch this because every layout hits it when deciding whether + // to show the silo/system picker. It's also fetched by the SystemLayout + // loader to figure out whether to 404, but RQ dedupes the request. + apiQueryClient.prefetchQuery('systemPolicyView', {}), + ]) } diff --git a/app/pages/LoginPage.tsx b/app/pages/LoginPage.tsx index a090adee4e..095dcc27f1 100644 --- a/app/pages/LoginPage.tsx +++ b/app/pages/LoginPage.tsx @@ -3,6 +3,8 @@ import { useNavigate, useSearchParams } from 'react-router-dom' import { useApiMutation } from '@oxide/api' import { Button, Success16Icon, Warning12Icon } from '@oxide/ui' +import { pb } from 'app/util/path-builder' + import { useToast } from '../hooks' /** @@ -28,7 +30,7 @@ export default function LoginPage() { title: 'Logged in', icon: , }) - navigate(searchParams.get('state') || '/') + navigate(searchParams.get('state') || pb.orgs()) }, onError: () => { addToast({ diff --git a/app/pages/__tests__/top-bar.e2e.ts b/app/pages/__tests__/top-bar.e2e.ts new file mode 100644 index 0000000000..b97d6daadd --- /dev/null +++ b/app/pages/__tests__/top-bar.e2e.ts @@ -0,0 +1,16 @@ +import { test } from '@playwright/test' + +import { expectSimultaneous } from 'app/test/e2e' +import { pb } from 'app/util/path-builder' + +test('Silo/system picker does not pop in', async ({ page }) => { + await page.goto(pb.orgs()) + + // make sure the system policy call is prefetched properly so that the + // silo/system picker doesn't pop in. if this turns out to be flaky, just + // throw it out. it's extra as hell + await expectSimultaneous(page, [ + 'role=button[name="Switch between system and silo"]', + 'role=button[name="Switch organization"]', + ]) +}) diff --git a/app/routes.tsx b/app/routes.tsx index 5ec3636b9a..a385d9070f 100644 --- a/app/routes.tsx +++ b/app/routes.tsx @@ -10,7 +10,7 @@ import RootLayout from './layouts/RootLayout' import SettingsLayout from './layouts/SettingsLayout' import SiloLayout from './layouts/SiloLayout' import SystemLayout from './layouts/SystemLayout' -import { prefetchSessionMe } from './layouts/helpers' +import { userLoader } from './layouts/helpers' import DeviceAuthSuccessPage from './pages/DeviceAuthSuccessPage' import DeviceAuthVerifyPage from './pages/DeviceAuthVerifyPage' import LoginPage from './pages/LoginPage' @@ -55,7 +55,7 @@ export const routes = createRoutesFromElements( {/* This wraps all routes that are supposed to be authenticated */} - }> + }> }> } /> } handle={{ crumb: 'Profile' }} /> @@ -73,7 +73,7 @@ export const routes = createRoutesFromElements( } handle={{ crumb: 'Hotkeys' }} /> - }> + } loader={SystemLayout.loader}> } loader={SilosPage.loader} /> { + const start = Date.now() + await page.locator(selector).waitFor() + return Date.now() - start +} + +/** + * Assert two elements appeared within 20ms of each other + */ +export async function expectSimultaneous(page: Page, selectors: [string, string]) { + const [t1, t2] = await Promise.all(selectors.map((sel) => timeToAppear(page, sel))) + expect(Math.abs(t1 - t2)).toBeLessThan(20) +} diff --git a/libs/api-mocks/msw/handlers.ts b/libs/api-mocks/msw/handlers.ts index a807ce4f3b..b73430eb9f 100644 --- a/libs/api-mocks/msw/handlers.ts +++ b/libs/api-mocks/msw/handlers.ts @@ -5,6 +5,7 @@ import { pick, sortBy } from '@oxide/util' import type { Json } from '../json-type' import { genCumulativeI64Data, genI64Data } from '../metrics' +import { FLEET_ID } from '../role-assignment' import { serial } from '../serial' import { sessionMe } from '../session' import { defaultSilo } from '../silo' @@ -53,6 +54,10 @@ const unavailableBody = { error_code: 'ServiceUnavailable' } as const type Unavailable = typeof unavailableBody const unavailableErr = json(unavailableBody, { status: 503 }) +const forbiddenBody = { error_code: 'Forbidden' } as const +type Forbidden = typeof forbiddenBody +// const forbiddenErr = json(forbiddenBody, { status: 403 }) + const badRequest = (msg: string) => compose( context.status(400), @@ -63,8 +68,8 @@ const badRequest = (msg: string) => }) ) -type GetErr = NotFound | Unavailable -type PostErr = AlreadyExists | NotFound +type GetErr = NotFound | Unavailable | Forbidden +type PostErr = AlreadyExists | NotFound | Forbidden export const handlers = [ rest.get('/session/me', (req, res) => res(json(sessionMe))), @@ -1139,6 +1144,19 @@ export const handlers = [ } ), + rest.get | GetErr>( + '/system/policy', + (_req, res) => { + // manually uncomment to test non-operator user + // TODO: figure out authz in the mock server ugh + // return res(forbiddenErr) + const role_assignments = db.roleAssignments + .filter((r) => r.resource_type === 'fleet' && r.resource_id === FLEET_ID) + .map((r) => pick(r, 'identity_id', 'identity_type', 'role_name')) + return res(json({ role_assignments })) + } + ), + rest.get | GetErr>('/users', (req, res) => { return res(json(paginated(req.url.search, db.users))) }), diff --git a/libs/api-mocks/role-assignment.ts b/libs/api-mocks/role-assignment.ts index 7f008648d1..a565f33110 100644 --- a/libs/api-mocks/role-assignment.ts +++ b/libs/api-mocks/role-assignment.ts @@ -1,4 +1,5 @@ import type { + FleetRoleRoleAssignment, OrganizationRoleRoleAssignment, ProjectRoleRoleAssignment, SiloRoleRoleAssignment, @@ -18,12 +19,23 @@ import { userGroup1, userGroup2 } from './user-group' // assignments and then collecting them into a policy object at request time. // See https://github.com/oxidecomputer/omicron/issues/1165 type DbRoleAssignment = { resource_id: string } & ( - | ({ resource_type: 'project' } & Json) - | ({ resource_type: 'organization' } & Json) + | ({ resource_type: 'fleet' } & Json) | ({ resource_type: 'silo' } & Json) + | ({ resource_type: 'organization' } & Json) + | ({ resource_type: 'project' } & Json) ) +// this is hard-coded in the API. there can only be one +export const FLEET_ID = '001de000-1334-4000-8000-000000000000' + export const roleAssignments: DbRoleAssignment[] = [ + { + resource_type: 'fleet', + resource_id: FLEET_ID, + identity_id: user1.id, + identity_type: 'silo_user', + role_name: 'admin', + }, { resource_type: 'silo', resource_id: defaultSilo.id, diff --git a/libs/api/index.ts b/libs/api/index.ts index 87cde962b2..5329506014 100644 --- a/libs/api/index.ts +++ b/libs/api/index.ts @@ -26,6 +26,7 @@ export const queryClient = new QueryClient({ queries: { retry: false, staleTime: 10000, + refetchOnWindowFocus: false, }, }, }) diff --git a/package.json b/package.json index be1bb24140..6121026165 100644 --- a/package.json +++ b/package.json @@ -34,7 +34,7 @@ "@reach/menu-button": "^0.16.2", "@reach/tabs": "^0.16.4", "@react-spring/web": "^9.4.5", - "@tanstack/react-query": "^4.0.10", + "@tanstack/react-query": "^4.12.0", "@tanstack/react-table": "^8.5.11", "classnames": "^2.3.1", "date-fns": "^2.28.0", diff --git a/yarn.lock b/yarn.lock index e0cac66049..d359ff16fb 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2375,18 +2375,17 @@ "@svgr/hast-util-to-babel-ast" "^6.3.1" svg-parser "^2.0.4" -"@tanstack/query-core@^4.0.0-beta.1": - version "4.0.5" - resolved "https://registry.yarnpkg.com/@tanstack/query-core/-/query-core-4.0.5.tgz#70c7275a9f1fbc8e6520d895991f9bace0fd5bb8" - integrity sha512-QOJ2gLbwlf8p0487pMey6vv8EF5X2ib1zINayaD7mb9/LibUtXmZ12uJgTqcnjgNY/4tWZn5qJnEk2ePG5AVGA== +"@tanstack/query-core@4.12.0": + version "4.12.0" + resolved "https://registry.yarnpkg.com/@tanstack/query-core/-/query-core-4.12.0.tgz#0e96adcfe182efc4ea4c21802f7596d56c6cd60a" + integrity sha512-KEiFPNLMFByhNL2s6RBFL6Z5cNdwwQzFpW/II3GY+rEuQ343ZEoVyQ48zlUXXkEkbamQFIFg2onM8Pxf0Yo01A== -"@tanstack/react-query@^4.0.10": - version "4.0.10" - resolved "https://registry.yarnpkg.com/@tanstack/react-query/-/react-query-4.0.10.tgz#92c71a2632c06450d848d4964959bd216cde03c0" - integrity sha512-Wn5QhZUE5wvr6rGClV7KeQIUsdTmYR9mgmMZen7DSRWauHW2UTynFg3Kkf6pw+XlxxOLsyLWwz/Q6q1lSpM3TQ== +"@tanstack/react-query@^4.12.0": + version "4.12.0" + resolved "https://registry.yarnpkg.com/@tanstack/react-query/-/react-query-4.12.0.tgz#2cb233ef1ccf7537aeed61ca171fc3dcc52d9c57" + integrity sha512-prchV1q+CJ0ZVo8Rts2cOF3azDfQizZZySmH6XXsXRcPTbir0sgb9fp0vY/5l5ZkSYjTvWt/OL8WQhAhYMSvrA== dependencies: - "@tanstack/query-core" "^4.0.0-beta.1" - "@types/use-sync-external-store" "^0.0.3" + "@tanstack/query-core" "4.12.0" use-sync-external-store "^1.2.0" "@tanstack/react-table@^8.5.11": @@ -2771,11 +2770,6 @@ dependencies: "@types/node" "*" -"@types/use-sync-external-store@^0.0.3": - version "0.0.3" - resolved "https://registry.yarnpkg.com/@types/use-sync-external-store/-/use-sync-external-store-0.0.3.tgz#b6725d5f4af24ace33b36fafd295136e75509f43" - integrity sha512-EwmlvuaxPNej9+T4v5AuBPJa2x2UOJVdjCtDHgcDqitUeOtjnJKJ+apYjVcAoBEMjKW1VVFGZLUb5+qqa09XFA== - "@types/uuid@^8.3.0": version "8.3.0" resolved "https://registry.npmjs.org/@types/uuid/-/uuid-8.3.0.tgz"