From 78c6c17c9e7348d3e049c21640f79ea0cb957a9f Mon Sep 17 00:00:00 2001 From: David Crespo Date: Thu, 30 Oct 2025 12:10:32 -0500 Subject: [PATCH 1/6] handle 403 on scim token list --- app/pages/system/silos/SiloScimTab.tsx | 42 +++++++++++++++++++------- mock-api/msw/handlers.ts | 9 +++--- mock-api/msw/util.ts | 14 +++++++++ test/e2e/scim-tokens.e2e.ts | 30 ++++++++++++++++-- 4 files changed, 77 insertions(+), 18 deletions(-) diff --git a/app/pages/system/silos/SiloScimTab.tsx b/app/pages/system/silos/SiloScimTab.tsx index 2ec048e53..7f55c3dcc 100644 --- a/app/pages/system/silos/SiloScimTab.tsx +++ b/app/pages/system/silos/SiloScimTab.tsx @@ -6,6 +6,7 @@ * Copyright Oxide Computer Company */ +import { useQuery } from '@tanstack/react-query' import { createColumnHelper, getCoreRowModel, useReactTable } from '@tanstack/react-table' import { useCallback, useMemo, useState } from 'react' import { type LoaderFunctionArgs } from 'react-router' @@ -14,9 +15,10 @@ import { AccessToken24Icon } from '@oxide/design-system/icons/react' import { Badge } from '@oxide/design-system/ui' import { + apiqErrorsAllowed, apiQueryClient, + queryClient, useApiMutation, - usePrefetchedApiQuery, type ScimClientBearerToken, } from '~/api' import { makeCrumb } from '~/hooks/use-crumbs' @@ -50,21 +52,29 @@ const EmptyState = () => ( export async function clientLoader({ params }: LoaderFunctionArgs) { const { silo } = getSiloSelector(params) - await apiQueryClient.prefetchQuery('scimTokenList', { query: { silo } }) + // Use errors-allowed approach so 403s don't throw and break the loader + await queryClient.prefetchQuery(apiqErrorsAllowed('scimTokenList', { query: { silo } })) return null } export default function SiloScimTab() { const siloSelector = useSiloSelector() - const { data } = usePrefetchedApiQuery('scimTokenList', { - query: { silo: siloSelector.silo }, - }) + const { data: queryResult } = useQuery( + apiqErrorsAllowed('scimTokenList', { + query: { silo: siloSelector.silo }, + }) + ) + + // Check if we got a 403 error + const is403 = queryResult?.type === 'error' && queryResult.data.statusCode === 403 // Order tokens by creation date, oldest first - const tokens = useMemo( - () => [...data].sort((a, b) => a.timeCreated.getTime() - b.timeCreated.getTime()), - [data] - ) + const tokens = useMemo(() => { + if (!queryResult || queryResult.type === 'error') return [] + return [...queryResult.data].sort( + (a, b) => a.timeCreated.getTime() - b.timeCreated.getTime() + ) + }, [queryResult]) const [showCreateModal, setShowCreateModal] = useState(false) const [createdToken, setCreatedToken] = useState<{ @@ -139,10 +149,20 @@ export default function SiloScimTab() { titleId="scim-tokens-label" description="Tokens for authenticating requests to SCIM endpoints" > - setShowCreateModal(true)}>Create token + {!is403 && ( + setShowCreateModal(true)}> + Create token + + )} - {tokens.length === 0 ? ( + {is403 ? ( + } + title="You do not have permission to view SCIM tokens" + body="Only fleet admins and silo admins can view and manage SCIM tokens for this silo" + /> + ) : tokens.length === 0 ? ( ) : ( t.siloId === silo.id) @@ -1901,8 +1902,8 @@ export const handlers = makeHandlers({ return tokens }, scimTokenCreate({ query, cookies }) { - requireFleetCollab(cookies) const silo = lookup.silo({ silo: query.silo }) + requireFleetAdminOrSiloAdmin(cookies, silo.id) const newToken: Json = { id: uuid(), @@ -1917,15 +1918,15 @@ export const handlers = makeHandlers({ return json(newToken, { status: 201 }) }, scimTokenView({ path, cookies }) { - requireFleetViewer(cookies) const token = lookupById(db.scimTokens, path.tokenId) + requireFleetAdminOrSiloAdmin(cookies, token.siloId) // Strip out siloId before returning const { siloId: _siloId, ...tokenResponse } = token return tokenResponse }, scimTokenDelete({ path, cookies }) { - requireFleetCollab(cookies) const token = lookupById(db.scimTokens, path.tokenId) + requireFleetAdminOrSiloAdmin(cookies, token.siloId) db.scimTokens = db.scimTokens.filter((t) => t.id !== token.id) return 204 }, diff --git a/mock-api/msw/util.ts b/mock-api/msw/util.ts index 5b605725b..741d99733 100644 --- a/mock-api/msw/util.ts +++ b/mock-api/msw/util.ts @@ -359,6 +359,20 @@ export function requireFleetAdmin(cookies: Record) { requireRole(cookies, 'fleet', FLEET_ID, 'admin') } +/** + * Determine whether current user has fleet admin OR silo admin on a specific + * silo. Used for SCIM token operations. Do nothing if yes, throw 403 if no. + */ +export function requireFleetAdminOrSiloAdmin( + cookies: Record, + siloId: string +) { + const user = currentUser(cookies) + const hasFleetAdmin = userHasRole(user, 'fleet', FLEET_ID, 'admin') + const hasSiloAdmin = userHasRole(user, 'silo', siloId, 'admin') + if (!hasFleetAdmin && !hasSiloAdmin) throw forbiddenErr() +} + /** * Determine whether current user has a role on a resource by looking roles * for the user as well as for the user's groups. Do nothing if yes, throw 403 diff --git a/test/e2e/scim-tokens.e2e.ts b/test/e2e/scim-tokens.e2e.ts index be64373fb..93bacf214 100644 --- a/test/e2e/scim-tokens.e2e.ts +++ b/test/e2e/scim-tokens.e2e.ts @@ -5,9 +5,15 @@ * * Copyright Oxide Computer Company */ -import { expect, test } from '@playwright/test' - -import { clickRowAction, expectNotVisible, expectRowVisible, expectVisible } from './utils' +import { + clickRowAction, + expect, + expectNotVisible, + expectRowVisible, + expectVisible, + getPageAsUser, + test, +} from './utils' test('SCIM tokens tab', async ({ page }) => { await page.goto('/system/silos/maze-war/scim') @@ -101,3 +107,21 @@ test('Delete SCIM token', async ({ page }) => { page.getByText('Create a token to see it here'), ]) }) + +test('Fleet viewer without silo admin cannot view SCIM tokens', async ({ browser }) => { + // Jane Austen is a fleet viewer but not a silo admin on maze-war + const page = await getPageAsUser(browser, 'Jane Austen') + await page.goto('/system/silos/maze-war/scim') + + // Should see permission denied message + await expectVisible(page, [ + page.getByRole('heading', { name: 'You do not have permission to view SCIM tokens' }), + page.getByText('Only fleet admins and silo admins can view and manage SCIM tokens'), + ]) + + // Create token button should not be visible + await expect(page.getByRole('button', { name: 'Create token' })).toBeHidden() + + // Table should not be visible + await expect(page.getByRole('table', { name: 'SCIM Tokens' })).toBeHidden() +}) From 0f7493c51458697fb18fc800e150a4960ca21f6c Mon Sep 17 00:00:00 2001 From: David Crespo Date: Thu, 30 Oct 2025 13:38:24 -0500 Subject: [PATCH 2/6] cleanup, center empty message, text-balance title --- app/pages/system/silos/SiloScimTab.tsx | 27 +++++++++++------------ app/ui/lib/EmptyMessage.tsx | 4 ++-- test/e2e/scim-tokens.e2e.ts | 30 ++++++++++++++------------ 3 files changed, 30 insertions(+), 31 deletions(-) diff --git a/app/pages/system/silos/SiloScimTab.tsx b/app/pages/system/silos/SiloScimTab.tsx index 7f55c3dcc..c0d736eb7 100644 --- a/app/pages/system/silos/SiloScimTab.tsx +++ b/app/pages/system/silos/SiloScimTab.tsx @@ -6,7 +6,6 @@ * Copyright Oxide Computer Company */ -import { useQuery } from '@tanstack/react-query' import { createColumnHelper, getCoreRowModel, useReactTable } from '@tanstack/react-table' import { useCallback, useMemo, useState } from 'react' import { type LoaderFunctionArgs } from 'react-router' @@ -19,6 +18,7 @@ import { apiQueryClient, queryClient, useApiMutation, + usePrefetchedQuery, type ScimClientBearerToken, } from '~/api' import { makeCrumb } from '~/hooks/use-crumbs' @@ -59,10 +59,8 @@ export async function clientLoader({ params }: LoaderFunctionArgs) { export default function SiloScimTab() { const siloSelector = useSiloSelector() - const { data: queryResult } = useQuery( - apiqErrorsAllowed('scimTokenList', { - query: { silo: siloSelector.silo }, - }) + const { data: queryResult } = usePrefetchedQuery( + apiqErrorsAllowed('scimTokenList', { query: siloSelector }) ) // Check if we got a 403 error @@ -96,16 +94,13 @@ export default function SiloScimTab() { label: 'Delete', onActivate: confirmDelete({ doDelete: () => - deleteToken.mutateAsync({ - path: { tokenId: token.id }, - query: { silo: siloSelector.silo }, - }), + deleteToken.mutateAsync({ path: { tokenId: token.id }, query: siloSelector }), resourceKind: 'SCIM token', label: token.id, }), }, ], - [deleteToken, siloSelector.silo] + [deleteToken, siloSelector] ) const staticColumns = useMemo( @@ -157,11 +152,13 @@ export default function SiloScimTab() { {is403 ? ( - } - title="You do not have permission to view SCIM tokens" - body="Only fleet admins and silo admins can view and manage SCIM tokens for this silo" - /> + + } + title="You do not have permission to view SCIM tokens" + body="Only fleet and silo admins can manage SCIM tokens for this silo" + /> + ) : tokens.length === 0 ? ( ) : ( diff --git a/app/ui/lib/EmptyMessage.tsx b/app/ui/lib/EmptyMessage.tsx index 337472ccb..0595f04f4 100644 --- a/app/ui/lib/EmptyMessage.tsx +++ b/app/ui/lib/EmptyMessage.tsx @@ -43,11 +43,11 @@ export function EmptyMessage(props: Props) { return (
{props.icon && ( -
+
{props.icon}
)} -

{props.title}

+

{props.title}

{typeof props.body === 'string' ? {props.body} : props.body} {button}
diff --git a/test/e2e/scim-tokens.e2e.ts b/test/e2e/scim-tokens.e2e.ts index 93bacf214..d7aaa3c0e 100644 --- a/test/e2e/scim-tokens.e2e.ts +++ b/test/e2e/scim-tokens.e2e.ts @@ -15,6 +15,9 @@ import { test, } from './utils' +const tokenId1 = 'a1b2c3d4…34567890' +const tokenId2 = 'b2c3d4e5…45678901' + test('SCIM tokens tab', async ({ page }) => { await page.goto('/system/silos/maze-war/scim') @@ -23,8 +26,8 @@ test('SCIM tokens tab', async ({ page }) => { const table = page.getByRole('table', { name: 'SCIM Tokens' }) // Check that existing tokens are visible - await expectRowVisible(table, { ID: 'a1b2c3d4…34567890' }) - await expectRowVisible(table, { ID: 'b2c3d4e5…45678901' }) + await expectRowVisible(table, { ID: tokenId1 }) + await expectRowVisible(table, { ID: tokenId2 }) }) test('Create SCIM token', async ({ page }) => { @@ -108,20 +111,19 @@ test('Delete SCIM token', async ({ page }) => { ]) }) -test('Fleet viewer without silo admin cannot view SCIM tokens', async ({ browser }) => { - // Jane Austen is a fleet viewer but not a silo admin on maze-war - const page = await getPageAsUser(browser, 'Jane Austen') +test('Only fleet or silo admin can view SCIM tokens', async ({ page, browser }) => { await page.goto('/system/silos/maze-war/scim') + await expect(page.getByText(tokenId1)).toBeVisible() - // Should see permission denied message - await expectVisible(page, [ - page.getByRole('heading', { name: 'You do not have permission to view SCIM tokens' }), - page.getByText('Only fleet admins and silo admins can view and manage SCIM tokens'), - ]) + // Jane Austen is a fleet viewer but not a silo admin on maze-war + const page2 = await getPageAsUser(browser, 'Jane Austen') + await page2.goto('/system/silos/maze-war/scim') - // Create token button should not be visible - await expect(page.getByRole('button', { name: 'Create token' })).toBeHidden() + await expect( + page2.getByRole('heading', { name: 'You do not have permission to view SCIM tokens' }) + ).toBeVisible() + await expect(page2.getByRole('button', { name: 'Create token' })).toBeHidden() - // Table should not be visible - await expect(page.getByRole('table', { name: 'SCIM Tokens' })).toBeHidden() + // Tokens should not be visible + await expect(page2.getByText(tokenId1)).toBeHidden() }) From e7453f7a04d247ee58d01278230234ccf727ad91 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Thu, 30 Oct 2025 14:42:07 -0500 Subject: [PATCH 3/6] fix implicit any coming from apiqErrorsAllowed --- app/api/hooks.ts | 4 ++-- app/pages/system/silos/SiloScimTab.tsx | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/api/hooks.ts b/app/api/hooks.ts index 510bdc048..f812e70b4 100644 --- a/app/api/hooks.ts +++ b/app/api/hooks.ts @@ -245,8 +245,8 @@ export const getApiQueryOptionsErrorsAllowed = queryFn: ({ signal }) => api[method](params, { signal }) .then(handleResult(method)) - .then((data) => ({ type: 'success' as const, data })) - .catch((data) => ({ type: 'error' as const, data })), + .then((data: Result) => ({ type: 'success' as const, data })) + .catch((data: ApiError) => ({ type: 'error' as const, data })), ...options, }) diff --git a/app/pages/system/silos/SiloScimTab.tsx b/app/pages/system/silos/SiloScimTab.tsx index c0d736eb7..9e34645b7 100644 --- a/app/pages/system/silos/SiloScimTab.tsx +++ b/app/pages/system/silos/SiloScimTab.tsx @@ -64,7 +64,7 @@ export default function SiloScimTab() { ) // Check if we got a 403 error - const is403 = queryResult?.type === 'error' && queryResult.data.statusCode === 403 + const is403 = queryResult.type === 'error' && queryResult.data.statusCode === 403 // Order tokens by creation date, oldest first const tokens = useMemo(() => { From 6f89b80d4954b58f095dead894a9931f727731ba Mon Sep 17 00:00:00 2001 From: David Crespo Date: Thu, 30 Oct 2025 15:19:22 -0500 Subject: [PATCH 4/6] rearrange some stuff --- app/pages/system/silos/SiloScimTab.tsx | 202 +++++++++++-------------- 1 file changed, 91 insertions(+), 111 deletions(-) diff --git a/app/pages/system/silos/SiloScimTab.tsx b/app/pages/system/silos/SiloScimTab.tsx index 9e34645b7..0415da5a7 100644 --- a/app/pages/system/silos/SiloScimTab.tsx +++ b/app/pages/system/silos/SiloScimTab.tsx @@ -9,6 +9,8 @@ import { createColumnHelper, getCoreRowModel, useReactTable } from '@tanstack/react-table' import { useCallback, useMemo, useState } from 'react' import { type LoaderFunctionArgs } from 'react-router' +import * as R from 'remeda' +import { match } from 'ts-pattern' import { AccessToken24Icon } from '@oxide/design-system/icons/react' import { Badge } from '@oxide/design-system/ui' @@ -20,6 +22,7 @@ import { useApiMutation, usePrefetchedQuery, type ScimClientBearerToken, + type ScimClientBearerTokenValue, } from '~/api' import { makeCrumb } from '~/hooks/use-crumbs' import { getSiloSelector, useSiloSelector } from '~/hooks/use-params' @@ -38,7 +41,24 @@ import { Modal } from '~/ui/lib/Modal' import { TableEmptyBox } from '~/ui/lib/Table' import { Truncate } from '~/ui/lib/Truncate' +export const handle = makeCrumb('SCIM') + const colHelper = createColumnHelper() +const staticColumns = [ + colHelper.accessor('id', { + header: 'ID', + cell: (info) => , + }), + colHelper.accessor('timeCreated', Columns.timeCreated), + colHelper.accessor('timeExpires', { + header: 'Expires', + cell: (info) => { + const expires = info.getValue() + return expires ? : Never + }, + meta: { thClassName: 'lg:w-1/4' }, + }), +] const EmptyState = () => ( @@ -57,37 +77,84 @@ export async function clientLoader({ params }: LoaderFunctionArgs) { return null } +type ModalState = + | { kind: 'create' } + | { kind: 'created'; token: ScimClientBearerTokenValue } + | false + export default function SiloScimTab() { const siloSelector = useSiloSelector() - const { data: queryResult } = usePrefetchedQuery( + const { data: tokensResult } = usePrefetchedQuery( apiqErrorsAllowed('scimTokenList', { query: siloSelector }) ) - // Check if we got a 403 error - const is403 = queryResult.type === 'error' && queryResult.data.statusCode === 403 + const [modalState, setModalState] = useState(false) - // Order tokens by creation date, oldest first - const tokens = useMemo(() => { - if (!queryResult || queryResult.type === 'error') return [] - return [...queryResult.data].sort( - (a, b) => a.timeCreated.getTime() - b.timeCreated.getTime() - ) - }, [queryResult]) + return ( + <> + + + { + // assume that if you can see the tokens, you can create tokens + tokensResult.type === 'success' && ( + setModalState({ kind: 'create' })}> + Create token + + ) + } + + + {match(tokensResult) + .with({ type: 'error' }, () => ( + + } + title="You do not have permission to view SCIM tokens" + body="Only fleet and silo admins can manage SCIM tokens for this silo" + /> + + )) + .with({ type: 'success' }, ({ data }) => ) + .exhaustive()} + + {/* TODO: put this back! + + + */} + - const [showCreateModal, setShowCreateModal] = useState(false) - const [createdToken, setCreatedToken] = useState<{ - id: string - bearerToken: string - timeCreated: Date - timeExpires?: Date | null - } | null>(null) + {match(modalState) + .with({ kind: 'create' }, () => ( + setModalState(false)} + onSuccess={(token) => setModalState({ kind: 'created', token })} + /> + )) + .with({ kind: 'created' }, ({ token }) => ( + setModalState(false)} /> + )) + .with(false, () => null) + .exhaustive()} + + ) +} +function TokensTable({ tokens }: { tokens: ScimClientBearerToken[] }) { + const siloSelector = useSiloSelector() const deleteToken = useApiMutation('scimTokenDelete', { onSuccess() { - apiQueryClient.invalidateQueries('scimTokenList') + queryClient.invalidateEndpoint('scimTokenList') }, }) + // Order tokens by creation date, oldest first + const sortedTokens = useMemo(() => R.sortBy(tokens, (a) => a.timeCreated), [tokens]) + const makeActions = useCallback( (token: ScimClientBearerToken): MenuAction[] => [ { @@ -103,98 +170,21 @@ export default function SiloScimTab() { [deleteToken, siloSelector] ) - const staticColumns = useMemo( - () => [ - colHelper.accessor('id', { - header: 'ID', - cell: (info) => ( - - ), - }), - colHelper.accessor('timeCreated', Columns.timeCreated), - colHelper.accessor('timeExpires', { - header: 'Expires', - cell: (info) => { - const expires = info.getValue() - return expires ? ( - - ) : ( - Never - ) - }, - meta: { thClassName: 'lg:w-1/4' }, - }), - ], - [] - ) - const columns = useColsWithActions(staticColumns, makeActions, 'Copy token ID') const table = useReactTable({ - data: tokens, + data: sortedTokens, columns, getCoreRowModel: getCoreRowModel(), }) - // const { href, linkText } = docLinks.scim - return ( - <> - - - {!is403 && ( - setShowCreateModal(true)}> - Create token - - )} - - - {is403 ? ( - - } - title="You do not have permission to view SCIM tokens" - body="Only fleet and silo admins can manage SCIM tokens for this silo" - /> - - ) : tokens.length === 0 ? ( - - ) : ( -
- )} - - {/* TODO: put this back! - - - */} - - {showCreateModal && ( - setShowCreateModal(false)} - onSuccess={(token) => { - setShowCreateModal(false) - setCreatedToken(token) - }} - /> - )} + if (sortedTokens.length === 0) return - {createdToken && ( - setCreatedToken(null)} /> - )} - + return ( +
) } -export const handle = makeCrumb('SCIM') - function CreateTokenModal({ siloSelector, onDismiss, @@ -202,12 +192,7 @@ function CreateTokenModal({ }: { siloSelector: { silo: string } onDismiss: () => void - onSuccess: (token: { - id: string - bearerToken: string - timeCreated: Date - timeExpires?: Date | null - }) => void + onSuccess: (token: ScimClientBearerTokenValue) => void }) { const createToken = useApiMutation('scimTokenCreate', { onSuccess(token) { @@ -243,12 +228,7 @@ function TokenCreatedModal({ token, onDismiss, }: { - token: { - id: string - bearerToken: string - timeCreated: Date - timeExpires?: Date | null - } + token: ScimClientBearerTokenValue onDismiss: () => void }) { return ( From f2a3bc697ce3bf15877ef904e0542fafd9e4e1d4 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Thu, 30 Oct 2025 15:49:01 -0500 Subject: [PATCH 5/6] add test for scim empty state --- mock-api/silo.ts | 6 ------ test/e2e/scim-tokens.e2e.ts | 9 +++++++++ 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/mock-api/silo.ts b/mock-api/silo.ts index e607c3399..77a2c1cb6 100644 --- a/mock-api/silo.ts +++ b/mock-api/silo.ts @@ -146,10 +146,4 @@ export const scimTokens: DbScimToken[] = [ time_expires: null, siloId: defaultSilo.id, }, - { - id: 'c3d4e5f6-a7b8-9012-cdef-123456789012', - time_created: new Date(2025, 8, 10).toISOString(), - time_expires: null, - siloId: silos[1].id, - }, ] diff --git a/test/e2e/scim-tokens.e2e.ts b/test/e2e/scim-tokens.e2e.ts index d7aaa3c0e..96a57d8b6 100644 --- a/test/e2e/scim-tokens.e2e.ts +++ b/test/e2e/scim-tokens.e2e.ts @@ -30,6 +30,15 @@ test('SCIM tokens tab', async ({ page }) => { await expectRowVisible(table, { ID: tokenId2 }) }) +test('SCIM tokens tab empty state', async ({ page }) => { + await page.goto('/system/silos/myriad/scim') + + const table = page.getByRole('table', { name: 'SCIM Tokens' }) + + await expect(table).toBeHidden() + await expect(page.getByRole('heading', { name: 'No SCIM tokens' })).toBeVisible() +}) + test('Create SCIM token', async ({ page }) => { await page.goto('/system/silos/maze-war/scim') From f24a63eaf1feb2744e182d619cb547f1b9b0e8cf Mon Sep 17 00:00:00 2001 From: David Crespo Date: Thu, 30 Oct 2025 15:53:00 -0500 Subject: [PATCH 6/6] remove one more apiQueryClient --- app/pages/system/silos/SiloScimTab.tsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/pages/system/silos/SiloScimTab.tsx b/app/pages/system/silos/SiloScimTab.tsx index 0415da5a7..1622534d9 100644 --- a/app/pages/system/silos/SiloScimTab.tsx +++ b/app/pages/system/silos/SiloScimTab.tsx @@ -17,7 +17,6 @@ import { Badge } from '@oxide/design-system/ui' import { apiqErrorsAllowed, - apiQueryClient, queryClient, useApiMutation, usePrefetchedQuery, @@ -196,7 +195,7 @@ function CreateTokenModal({ }) { const createToken = useApiMutation('scimTokenCreate', { onSuccess(token) { - apiQueryClient.invalidateQueries('scimTokenList') + queryClient.invalidateEndpoint('scimTokenList') onSuccess(token) }, onError(err) {