Better error for admin group name misconfig (#2671)

* better error for group misconfig

* no "hint:"

* much better look after a ben consultation

* move docs link to links.ts, update URL

Author: david-crespo

app/components/ErrorBoundary.tsx

@@ -5,30 +5,75 @@
  *
  * Copyright Oxide Computer Company
  */
+import { useQuery } from '@tanstack/react-query'
 import { ErrorBoundary as BaseErrorBoundary } from 'react-error-boundary'
 import { useRouteError } from 'react-router'
 
+import { apiq } from '~/api'
 import { type ApiError } from '~/api/errors'
+import { Message } from '~/ui/lib/Message'
+import { links } from '~/util/links'
 
 import { ErrorPage, NotFound } from './ErrorPage'
 
+const IdpMisconfig = () => (
+  <Message
+    variant="notice"
+    className="!mt-6"
+    showIcon={false}
+    content={
+      <p>
+        You are not in any groups and have no role on the silo. This usually means the
+        identity provider is not set up correctly. Read the{' '}
+        <a
+          href={links.troubleshootingAccess}
+          className="underline"
+          target="_blank"
+          rel="noreferrer"
+        >
+          docs
+        </a>{' '}
+        for more information.
+      </p>
+    }
+  />
+)
+
+function useDetectNoSiloRole(enabled: boolean): boolean {
+  // this is kind of a hail mary, so if any of this goes wrong we need to ignore it
+  const options = { enabled, throwOnError: false }
+  const { data: me } = useQuery(apiq('currentUserView', {}, options))
+  const { data: myGroups } = useQuery(apiq('currentUserGroups', {}, options))
+  const { data: siloPolicy } = useQuery(apiq('policyView', {}, options))
+
+  if (!me || !myGroups || !siloPolicy) return false
+
+  const noGroups = myGroups.items.length === 0
+  const hasDirectRoleOnSilo = siloPolicy.roleAssignments.some((r) => r.identityId === me.id)
+  return noGroups && !hasDirectRoleOnSilo
+}
+
 export const trigger404 = { type: 'error', statusCode: 404 }
 
 type Props = { error: Error | ApiError }
 
 function ErrorFallback({ error }: Props) {
   console.error(error)
+  const statusCode = 'statusCode' in error ? error.statusCode : undefined
+
+  // if the error is a 403, make API calls to check whether the user has any
+  // groups or any roles directly on the silo
+  const showIdpMisconfig = useDetectNoSiloRole(statusCode === 403)
 
-  if ('statusCode' in error && error.statusCode === 404) {
-    return <NotFound />
-  }
+  if (statusCode === 404) return <NotFound />
 
   return (
     <ErrorPage>
       <h1 className="text-sans-2xl">Something went wrong</h1>
       <p className="text-secondary">
         Please try again. If the problem persists, contact your administrator.
       </p>
+      {showIdpMisconfig && <IdpMisconfig />}
     </ErrorPage>
   )
 }

app/forms/image-upload.tsx

@@ -539,6 +539,7 @@ export function Component() {
           await onSubmit(values)
         } catch (e) {
           if (e !== ABORT_ERROR) {
+            console.error(e)
             setModalError('Something went wrong. Please try again.')
             // abort anything in flight in case
             cancelEverything()

app/pages/ProjectsPage.tsx

@@ -39,7 +39,11 @@ const EmptyState = () => (
 const projectList = getListQFn('projectList', {})
 
 export async function loader() {
-  await queryClient.prefetchQuery(projectList.optionsFn())
+  // fetchQuery instead of prefetchQuery means errors blow up here instead of
+  // waiting to hit the invariant in the useQuery call. We may end up doing this
+  // everywhere, but here in particular it is needed to trigger the silo group
+  // misconfig detection case in ErrorBoundary.
+  await queryClient.fetchQuery(projectList.optionsFn())
   return null
 }
 

app/ui/lib/Message.tsx

@@ -27,8 +27,7 @@ export interface MessageProps {
     text: string
     link: To
   }
-  // try to use icons from the ___12Icon set, rather than forcing a 16px or 24px icon
-  icon?: ReactElement
+  showIcon?: boolean
 }
 
 const defaultIcon: Record<Variant, ReactElement> = {
@@ -73,21 +72,21 @@ export const Message = ({
   className,
   variant = 'info',
   cta,
-  icon,
+  showIcon = true,
 }: MessageProps) => {
   return (
     <div
       className={cn(
-        'relative flex items-start overflow-hidden rounded-lg p-4 elevation-1',
+        'relative flex items-start gap-2.5 overflow-hidden rounded-lg p-4 elevation-1',
         color[variant],
         textColor[variant],
         className
       )}
     >
-      <div className="mt-[2px] flex [&>svg]:h-3 [&>svg]:w-3">
-        {icon || defaultIcon[variant]}
-      </div>
-      <div className="flex-1 pl-2.5">
+      {showIcon && (
+        <div className="mt-[2px] flex [&>svg]:h-3 [&>svg]:w-3">{defaultIcon[variant]}</div>
+      )}
+      <div className="flex-1">
         {title && <div className="text-sans-semi-md">{title}</div>}
         <div
           className={cn(

app/util/links.ts

@@ -50,6 +50,8 @@ export const links = {
   systemSiloDocs: 'https://docs.oxide.computer/guides/operator/silo-management',
   transitIpsDocs:
     'https://docs.oxide.computer/guides/configuring-guest-networking#_example_4_software_routing_tunnels',
+  troubleshootingAccess:
+    'https://docs.oxide.computer/guides/operator/faq#_how_do_i_fix_the_something_went_wrong_error',
   instancesDocs: 'https://docs.oxide.computer/guides/deploying-workloads',
   vpcsDocs: 'https://docs.oxide.computer/guides/configuring-guest-networking',
 }

mock-api/msw/handlers.ts

@@ -72,8 +72,14 @@ export const handlers = makeHandlers({
   loginLocal: ({ body: { password } }) => (password === 'bad' ? 401 : 200),
   groupList: (params) => paginated(params.query, db.userGroups),
   groupView: (params) => lookupById(db.userGroups, params.path.groupId),
-
-  projectList: (params) => paginated(params.query, db.projects),
+  projectList: ({ query, cookies }) => {
+    // this is used to test for the IdP misconfig situation where the user has
+    // no role on the silo (see error-pages.e2e.ts). requireRole checks for _at
+    // least_ viewer, and viewer is the weakest role, so checking for viewer
+    // effectively means "do I have any role at all"
+    requireRole(cookies, 'silo', defaultSilo.id, 'viewer')
+    return paginated(query, db.projects)
+  },
   projectCreate({ body }) {
     errIfExists(db.projects, { name: body.name }, 'project')
 
@@ -189,7 +195,7 @@ export const handlers = makeHandlers({
       throw 'Can only enter state importing_from_bulk_write from import_ready'
     }
 
-    await delay(1000) // slow it down for the tests
+    await delay(2000) // slow it down for the tests
 
     db.diskBulkImportState.set(disk.id, { blocks: {} })
     disk.state = { state: 'importing_from_bulk_writes' }
@@ -203,7 +209,7 @@ export const handlers = makeHandlers({
     if (disk.state.state !== 'importing_from_bulk_writes') {
       throw 'Can only stop import for disk in state importing_from_bulk_write'
     }
-    await delay(1000) // slow it down for the tests
+    await delay(2000) // slow it down for the tests
 
     db.diskBulkImportState.delete(disk.id)
     disk.state = { state: 'import_ready' }

test/e2e/error-pages.e2e.ts

@@ -7,6 +7,8 @@
  */
 import { expect, test } from '@playwright/test'
 
+import { getPageAsUser } from './utils'
+
 test('Shows 404 page when a resource is not found', async ({ page }) => {
   await page.goto('/nonexistent')
   await expect(page.locator('text=Page not found')).toBeVisible()
@@ -42,3 +44,10 @@ test('Shows something went wrong page on other errors', async ({ page }) => {
   // up at the right URL
   await expect(page).toHaveURL('/login')
 })
+
+test('error page for user with no groups or silo role', async ({ browser }) => {
+  const page = await getPageAsUser(browser, 'Jacob Klein')
+  await page.goto('/projects')
+  await expect(page.getByText('Something went wrong')).toBeVisible()
+  await expect(page.getByText('identity provider is not set up correctly')).toBeVisible()
+})

test/e2e/image-upload.e2e.ts

@@ -159,7 +159,8 @@ test.describe('Image upload', () => {
       await page.getByRole('button', { name: 'Cancel' }).click()
       await page.getByRole('link', { name: 'Disks' }).click()
       await expect(page.getByRole('cell', { name: 'disk-1', exact: true })).toBeVisible()
-      await expect(page.getByRole('cell', { name: 'tmp' })).toBeHidden()
+      // needs a little extra time for delete to go through
+      await expect(page.getByRole('cell', { name: 'tmp' })).toBeHidden({ timeout: 10000 })
     })
   }
 

test/e2e/utils.ts

@@ -133,11 +133,16 @@ export async function expectNoToast(page: Page, expectedText: string | RegExp) {
 }
 
 /**
- * Close toast and wait for it to fade out. For some reason it prevents things
- * from working, but only in tests as far as we can tell.
+ * Close first toast and wait for it to fade out. For some reason it prevents
+ * things from working, but only in tests as far as we can tell.
  */
 export async function closeToast(page: Page) {
-  await page.getByRole('button', { name: 'Dismiss notification' }).click()
+  // first() is a hack aimed at situations where we're testing an error
+  // response, which usually means we have an initial "creating..." toast
+  // followed by an error toast. Sometimes the error toast shows up so fast that
+  // we don't have time to close the first one. Without first(), this errors out
+  // because there are two toasts.
+  await page.getByRole('button', { name: 'Dismiss notification' }).first().click()
   await sleep(1000)
 }
 
@@ -185,7 +190,7 @@ export async function selectOption(
 
 export async function getPageAsUser(
   browser: Browser,
-  user: 'Hans Jonas' | 'Simone de Beauvoir'
+  user: 'Hans Jonas' | 'Simone de Beauvoir' | 'Jacob Klein'
 ): Promise<Page> {
   const browserContext = await browser.newContext()
   await browserContext.addCookies([