Hide system/silo picker and 404 on system pages if /system/policy 4…

…03s (#1232)

* show/hide silo picker in top bar based on fleet policy

* 404 if user tries to hit system routes without fleet viewer perm

* fix SystemLayout name

* today is the day to be less stupid. don't detect system route by looking at strings

* FINALLY figured out how to handle the top bar pickers in the layouts

* comments and naming tweaks

* let's be extra. test whether silo/system picker pops in

* delete the bad one

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) {

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 && <SiloSystemPicker isSystem={isSystem} key={0} />,
-    // 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 && <OrgPicker key={1} />,
-    projectName && <ProjectPicker key={2} />,
-  ].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).

app/components/TopBarPicker.tsx

@@ -104,7 +104,29 @@ const NoOrgLogo = () => (
   </div>
 )
 
-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 `<TopBar>`. Having
+ * `<SiloSystemPicker>` itself return `null` from render doesn't work because
+ * `<TopBar>` 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 ? <SiloSystemPicker value={value} /> : 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' ? (
     <TopBarPicker
       {...commonProps}
       category="System"
@@ -127,8 +149,6 @@ export function SiloSystemPicker({ isSystem }: { isSystem: boolean }) {
   )
 }
 
-// TODO: maybe don't filter out the currently selected one
-
 export function OrgPicker() {
   const { orgName } = useParams()
   const { data } = useApiQuery('organizationList', { limit: 20 })

app/layouts/OrgLayout.tsx

@@ -1,6 +1,7 @@
 import { Access16Icon, Divider, Folder16Icon, Organization16Icon } from '@oxide/ui'
 
 import { TopBar } from 'app/components/TopBar'
+import { OrgPicker, useSiloSystemPicker } from 'app/components/TopBarPicker'
 import { useRequiredParams } from 'app/hooks'
 import { pb } from 'app/util/path-builder'
 
@@ -12,7 +13,10 @@ const OrgLayout = () => {
 
   return (
     <PageContainer>
-      <TopBar />
+      <TopBar>
+        {useSiloSystemPicker('silo')}
+        <OrgPicker />
+      </TopBar>
       <Sidebar>
         <Sidebar.Nav>
           <NavLinkItem to={pb.orgs()} end>

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 (
     <PageContainer>
-      <TopBar />
+      <TopBar>
+        {useSiloSystemPicker('silo')}
+        <OrgPicker />
+        <ProjectPicker />
+      </TopBar>
       <Sidebar>
         <Sidebar.Nav>
           <NavLinkItem to={pb.projects({ orgName })} end>

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 (
     <PageContainer>
-      <TopBar />
+      <TopBar>
+        {useSiloSystemPicker('silo')}
+        <OrgPicker />
+      </TopBar>
       <Sidebar>
         <Sidebar.Nav>
           {/* TODO: what to link here? anything? */}

app/layouts/SiloLayout.tsx

@@ -2,14 +2,18 @@ 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'
 
 export default function SiloLayout() {
   return (
     <PageContainer>
-      <TopBar />
+      <TopBar>
+        {useSiloSystemPicker('silo')}
+        <OrgPicker />
+      </TopBar>
       <Sidebar>
         <Sidebar.Nav>
           <DocsLinkItem />

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 (
     <PageContainer>
-      <TopBar />
+      <TopBar>
+        {/* don't use the hook bc if we're here, this needs to show up */}
+        <SiloSystemPicker value="system" />
+      </TopBar>
       <Sidebar>
         <Sidebar.Nav>
           <DocsLinkItem />

app/layouts/helpers.tsx

@@ -24,6 +24,13 @@ export const ContentPane = () => (
   </div>
 )
 
-export async function prefetchSessionMe() {
-  await apiQueryClient.prefetchQuery('sessionMe', {})
+/** Loader for the `<Route>` 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', {}),
+  ])
 }

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: <Success16Icon />,
       })
-      navigate(searchParams.get('state') || '/')
+      navigate(searchParams.get('state') || pb.orgs())
     },
     onError: () => {
       addToast({

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"]',
+  ])
+})

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(
     </Route>
 
     {/* This wraps all routes that are supposed to be authenticated */}
-    <Route loader={prefetchSessionMe} errorElement={<RouterDataErrorBoundary />}>
+    <Route loader={userLoader} errorElement={<RouterDataErrorBoundary />}>
       <Route path="settings" handle={{ crumb: 'settings' }} element={<SettingsLayout />}>
         <Route index element={<Navigate to="profile" replace />} />
         <Route path="profile" element={<ProfilePage />} handle={{ crumb: 'Profile' }} />
@@ -73,7 +73,7 @@ export const routes = createRoutesFromElements(
         <Route path="hotkeys" element={<HotkeysPage />} handle={{ crumb: 'Hotkeys' }} />
       </Route>
 
-      <Route path="sys" element={<SystemLayout />}>
+      <Route path="sys" element={<SystemLayout />} loader={SystemLayout.loader}>
         <Route path="silos" element={<SilosPage />} loader={SilosPage.loader} />
         <Route
           path="silos-new"

app/test/e2e/utils.ts

@@ -77,3 +77,17 @@ export async function expectRowVisible(
     .poll(getRows)
     .toEqual(expect.arrayContaining([expect.objectContaining(expectedRow)]))
 }
+
+async function timeToAppear(page: Page, selector: string): Promise<number> {
+  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)
+}

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<never, never, Json<Api.FleetRolePolicy> | 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<never, never, Json<Api.UserResultsPage> | GetErr>('/users', (req, res) => {
     return res(json(paginated(req.url.search, db.users)))
   }),