fix(catalyst-ui,api): null-map crash on /users + /login open-redirect
qa-loop iter-4 cluster `users-page-null-map-and-open-redirect` —
TC-028/169/222 (P0) + TC-009 (P1 sec).
Sub-A (P0 regression): /users and /provision/{id}/users SPA pages
crashed with `TypeError: Cannot read properties of null (reading
'map')` rendering the error boundary. Root cause: the catalyst-api
`unstructuredToUserAccess` left `Spec.Applications` as a nil slice
when the source UserAccess CR omitted .spec.applications, which Go
serializes as `null` over JSON — and the React UserAccessListPage
called `applications.map(...)` directly. Fixes:
- api: initialize Spec.Applications = []userAccessAppGrantBody{}
in unstructuredToUserAccess so the wire shape is `[]` not `null`
- ui: defensively normalize each item in listUserAccess (api client)
so applications/keycloakGroups null-leaks never reach React
- ui: tolerate nulls in grantsSummary, UserAccessListPage items
rendering, and MembersList flattenForScope/grantForScope
- test: BE check that an empty list serializes as `"items":[]` and
that unstructuredToUserAccess emits `"applications":[]`
- test: FE renders without crashing when applications is null AND
when initialItems is null
Sub-B (P1 security CWE-601): TC-009 anonymous /dashboard visit
redirected to /login?next=//dashboard. The leading `//` is parsed
by the browser as a protocol-relative URL — an attacker could craft
`/login?next=//evil.com/path` and bounce victims off-origin after
sign-in. Fixes:
- new sanitizeNextParam in auth-gate: rejects empty / non-string,
embedded NUL or whitespace, backslashes, explicit URL schemes,
leading `//`, and any input not starting with a single `/`
- rootBeforeLoad: sanitize the deep-link `next` BEFORE the redirect
- loginRoute + loginVerifyRoute validateSearch: strip unsafe `next`
so URL-supplied attack payloads never reach the components
- VerifyPinPage: belt-and-suspenders sanitize at the consumer
point (`window.location.replace(target)`) so a future caller
bypassing validateSearch still can't smuggle an off-origin URL
- test: 7-case sanitizeNextParam coverage (empty, safe paths,
multi-slash, scheme-prefixed URLs, backslash variants, relative
paths, control chars / whitespace)
Files changed:
- products/catalyst/bootstrap/api/internal/handler/user_access.go
- products/catalyst/bootstrap/api/internal/handler/user_access_test.go
- products/catalyst/bootstrap/ui/src/app/auth-gate.ts (+ test)
- products/catalyst/bootstrap/ui/src/app/router.tsx
- products/catalyst/bootstrap/ui/src/pages/admin/rbac/membersListHelpers.ts (+ test)
- products/catalyst/bootstrap/ui/src/pages/admin/user-access/UserAccessListPage.tsx (+ test)
- products/catalyst/bootstrap/ui/src/pages/admin/user-access/userAccess.api.ts
- products/catalyst/bootstrap/ui/src/pages/auth/VerifyPinPage.tsx
Tests: 54 UI tests pass (auth-gate + membersListHelpers +
UserAccessListPage), all user_access handler Go tests pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
hatiyildiz
parent
9026bf6492
commit
0586f7d612
@ -442,8 +442,16 @@ func userAccessToUnstructured(req userAccessRequest) *unstructured.Unstructured
|
||||
}
|
||||
|
||||
func unstructuredToUserAccess(u *unstructured.Unstructured) userAccessItem {
|
||||
// Initialize slice fields as empty (not nil) so JSON serialization
|
||||
// emits `[]` rather than `null`. The UI renders `applications.map(...)`
|
||||
// directly — null would crash the page with a TypeError. Caught on
|
||||
// console.omantel.biz 2026-05-09 (qa-loop iter-4 cluster
|
||||
// `users-page-null-map-and-open-redirect`).
|
||||
out := userAccessItem{
|
||||
Name: u.GetName(),
|
||||
Spec: userAccessSpecBody{
|
||||
Applications: []userAccessAppGrantBody{},
|
||||
},
|
||||
}
|
||||
if ts := u.GetCreationTimestamp(); !ts.IsZero() {
|
||||
out.CreationTimestamp = ts.UTC().Format("2006-01-02T15:04:05Z")
|
||||
|
||||
@ -140,6 +140,51 @@ func TestListUserAccess_Empty(t *testing.T) {
|
||||
if len(out.Items) != 0 {
|
||||
t.Fatalf("expected empty list; got %d items", len(out.Items))
|
||||
}
|
||||
// qa-loop iter-4 cluster `users-page-null-map-and-open-redirect`:
|
||||
// the empty-list body MUST serialize the items field as `[]` (not
|
||||
// `null`) so the React UI's `items.map(...)` does not throw
|
||||
// `TypeError: Cannot read properties of null (reading 'map')`.
|
||||
rawBody := rec.Body.String()
|
||||
if !strings.Contains(rawBody, `"items":[]`) {
|
||||
t.Fatalf("expected items field to serialize as []; body=%s", rawBody)
|
||||
}
|
||||
if strings.Contains(rawBody, `"items":null`) {
|
||||
t.Fatalf("items field must never be null on the wire; body=%s", rawBody)
|
||||
}
|
||||
}
|
||||
|
||||
// TestUnstructuredToUserAccess_NilApplicationsBecomesEmpty proves the
|
||||
// Spec.Applications field initializes to an empty slice (not nil) so
|
||||
// the JSON encoder emits `"applications":[]` for an UserAccess CR
|
||||
// missing the field. Mirrors the FE-side normalizer in
|
||||
// `userAccess.api.ts:normalizeItem`. qa-loop iter-4 cluster
|
||||
// `users-page-null-map-and-open-redirect`.
|
||||
func TestUnstructuredToUserAccess_NilApplicationsBecomesEmpty(t *testing.T) {
|
||||
u := &unstructured.Unstructured{}
|
||||
u.SetAPIVersion("access.openova.io/v1alpha1")
|
||||
u.SetKind("UserAccess")
|
||||
u.SetName("missing-apps")
|
||||
// No spec at all — verify Applications is still [], not nil.
|
||||
|
||||
out := unstructuredToUserAccess(u)
|
||||
if out.Spec.Applications == nil {
|
||||
t.Fatalf("Spec.Applications must be initialized as empty slice (not nil)")
|
||||
}
|
||||
if len(out.Spec.Applications) != 0 {
|
||||
t.Fatalf("Spec.Applications should be empty; got %d", len(out.Spec.Applications))
|
||||
}
|
||||
// JSON-roundtrip — must not produce a `null`.
|
||||
body, err := json.Marshal(out)
|
||||
if err != nil {
|
||||
t.Fatalf("marshal: %v", err)
|
||||
}
|
||||
bodyStr := string(body)
|
||||
if !strings.Contains(bodyStr, `"applications":[]`) {
|
||||
t.Fatalf("expected applications:[]; got %s", bodyStr)
|
||||
}
|
||||
if strings.Contains(bodyStr, `"applications":null`) {
|
||||
t.Fatalf("applications must never be null; got %s", bodyStr)
|
||||
}
|
||||
}
|
||||
|
||||
func TestListUserAccess_Populated(t *testing.T) {
|
||||
|
||||
@ -4,6 +4,7 @@ import {
|
||||
isPublicPath,
|
||||
hasCatalystSession,
|
||||
probeWhoamiAndCacheMarker,
|
||||
sanitizeNextParam,
|
||||
PUBLIC_PATH_PREFIXES,
|
||||
} from './auth-gate'
|
||||
|
||||
@ -181,6 +182,69 @@ describe('probeWhoamiAndCacheMarker', () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe('sanitizeNextParam — open-redirect defense (CWE-601)', () => {
|
||||
// qa-loop iter-4 cluster `users-page-null-map-and-open-redirect` —
|
||||
// TC-009 surfaced /login?next=//dashboard with a leading `//` which
|
||||
// would let an attacker craft /login?next=//evil.com to bounce the
|
||||
// operator off-origin after sign-in.
|
||||
|
||||
it('returns undefined for undefined / empty / non-string input', () => {
|
||||
expect(sanitizeNextParam(undefined)).toBeUndefined()
|
||||
expect(sanitizeNextParam('')).toBeUndefined()
|
||||
expect(sanitizeNextParam(null)).toBeUndefined()
|
||||
expect(sanitizeNextParam(123)).toBeUndefined()
|
||||
expect(sanitizeNextParam({})).toBeUndefined()
|
||||
})
|
||||
|
||||
it('passes safe single-leading-slash paths through unchanged', () => {
|
||||
expect(sanitizeNextParam('/dashboard')).toBe('/dashboard')
|
||||
expect(sanitizeNextParam('/provision/d-1/users')).toBe('/provision/d-1/users')
|
||||
expect(sanitizeNextParam('/users/some-user@example.org?tab=grants')).toBe(
|
||||
'/users/some-user@example.org?tab=grants',
|
||||
)
|
||||
expect(sanitizeNextParam('/dashboard#anchor')).toBe('/dashboard#anchor')
|
||||
})
|
||||
|
||||
it('collapses leading multi-slashes to a single slash (TC-009 attack vector)', () => {
|
||||
// Protocol-relative URLs treated as host references by the browser
|
||||
expect(sanitizeNextParam('//dashboard')).toBeUndefined()
|
||||
expect(sanitizeNextParam('//evil.com/path')).toBeUndefined()
|
||||
expect(sanitizeNextParam('///foo')).toBeUndefined()
|
||||
expect(sanitizeNextParam('////a/b')).toBeUndefined()
|
||||
})
|
||||
|
||||
it('rejects absolute URLs with explicit schemes', () => {
|
||||
expect(sanitizeNextParam('http://evil.com/path')).toBeUndefined()
|
||||
expect(sanitizeNextParam('https://evil.com/path')).toBeUndefined()
|
||||
expect(sanitizeNextParam('javascript:alert(1)')).toBeUndefined()
|
||||
expect(sanitizeNextParam('data:text/html,<script>alert(1)</script>')).toBeUndefined()
|
||||
expect(sanitizeNextParam('file:///etc/passwd')).toBeUndefined()
|
||||
expect(sanitizeNextParam('vbscript:msgbox(1)')).toBeUndefined()
|
||||
})
|
||||
|
||||
it('rejects backslash-prefixed paths (browser quirk)', () => {
|
||||
// Some browsers normalize \ to / in URLs — catching `\\evil.com`
|
||||
// and `/\evil.com` as cousins of `//evil.com`.
|
||||
expect(sanitizeNextParam('\\\\evil.com/path')).toBeUndefined()
|
||||
expect(sanitizeNextParam('/\\evil.com')).toBeUndefined()
|
||||
expect(sanitizeNextParam('\\evil.com')).toBeUndefined()
|
||||
})
|
||||
|
||||
it('rejects strings that do not start with /', () => {
|
||||
expect(sanitizeNextParam('dashboard')).toBeUndefined()
|
||||
expect(sanitizeNextParam('./dashboard')).toBeUndefined()
|
||||
expect(sanitizeNextParam('../etc/passwd')).toBeUndefined()
|
||||
expect(sanitizeNextParam('mailto:attacker@evil.com')).toBeUndefined()
|
||||
})
|
||||
|
||||
it('rejects strings containing whitespace or control characters', () => {
|
||||
expect(sanitizeNextParam('/dashboard\n')).toBeUndefined()
|
||||
expect(sanitizeNextParam('/dashboard ')).toBeUndefined()
|
||||
expect(sanitizeNextParam(' /dashboard')).toBeUndefined()
|
||||
expect(sanitizeNextParam('/dash\x00board')).toBeUndefined()
|
||||
})
|
||||
})
|
||||
|
||||
describe('integration — anti-regression for #1090 cluster A2 bypass routes', () => {
|
||||
// For each of the 7 routes that bypassed the layout-only gate, the
|
||||
// rootBeforeLoad gate should now redirect to /login. We verify the
|
||||
|
||||
@ -39,6 +39,49 @@ export function canonicalisePath(pathname: string): string {
|
||||
return p.toLowerCase()
|
||||
}
|
||||
|
||||
/**
|
||||
* Sanitize a `next=` redirect target so it can NEVER point to an
|
||||
* external host (open-redirect class CWE-601).
|
||||
*
|
||||
* Rules (strictly reject anything ambiguous — paths only):
|
||||
* - If the input is empty / non-string → return undefined.
|
||||
* - Reject embedded NUL / control / whitespace characters.
|
||||
* - Reject explicit URL schemes (`http:`, `https:`, `javascript:`,
|
||||
* `data:`, `file:`, `vbscript:`, `mailto:`, etc).
|
||||
* - Reject backslashes anywhere (some browsers normalize `\` to
|
||||
* `/`, which would smuggle `\\evil.com` past a leading-slash
|
||||
* check).
|
||||
* - Reject anything that doesn't start with a single forward
|
||||
* slash. In particular `//evil.com/foo` is rejected — the
|
||||
* browser parses leading `//` as a protocol-relative authority
|
||||
* reference (host = evil.com).
|
||||
*
|
||||
* Returns the sanitized path, or `undefined` if the input is unsafe
|
||||
* — callers should fall back to the default landing page (e.g.
|
||||
* `/dashboard` or `/wizard`).
|
||||
*
|
||||
* qa-loop iter-4 cluster `users-page-null-map-and-open-redirect`
|
||||
* (TC-009 / 2026-05-09 routing matrix). The contract: post-login
|
||||
* navigation MUST land on the same origin under all circumstances.
|
||||
*/
|
||||
export function sanitizeNextParam(raw: unknown): string | undefined {
|
||||
if (typeof raw !== 'string' || raw.length === 0) return undefined
|
||||
// Reject embedded NULs / control chars / any whitespace.
|
||||
if (/[\x00-\x1f\s]/.test(raw)) return undefined
|
||||
// Reject backslashes anywhere — some browsers normalize `\` to `/`
|
||||
// before parsing, so `/\evil.com` and `\\evil.com` end up host
|
||||
// references at runtime.
|
||||
if (raw.indexOf('\\') !== -1) return undefined
|
||||
// Reject schemes (http:, https:, javascript:, data:, file:, mailto:,
|
||||
// vbscript:, etc.). A scheme is `<alpha>[<alpha>|<digit>|+|.|-]*:`.
|
||||
if (/^[a-zA-Z][a-zA-Z0-9+.-]*:/.test(raw)) return undefined
|
||||
// Require single-leading-slash absolute path. Reject leading `//`
|
||||
// (protocol-relative authority) and anything that doesn't start
|
||||
// with `/` (relative paths or bare strings).
|
||||
if (!raw.startsWith('/') || raw.startsWith('//')) return undefined
|
||||
return raw
|
||||
}
|
||||
|
||||
/**
|
||||
* Synchronous best-effort check that the operator has a session marker
|
||||
* cached in JS-readable storage.
|
||||
|
||||
@ -105,6 +105,7 @@ import {
|
||||
hasCatalystSession,
|
||||
isPublicPath,
|
||||
probeWhoamiAndCacheMarker,
|
||||
sanitizeNextParam,
|
||||
} from './auth-gate'
|
||||
|
||||
/**
|
||||
@ -161,10 +162,15 @@ async function rootBeforeLoad({ location }: { location: { pathname: string } })
|
||||
const whoami = await probeWhoamiAndCacheMarker(API_BASE)
|
||||
if (whoami === true) return
|
||||
if (whoami === null) return // 5xx / network error — fail open
|
||||
// 401 — genuinely unauthenticated. Redirect with deep-link.
|
||||
// 401 — genuinely unauthenticated. Redirect with deep-link, but
|
||||
// sanitize the `next` so we can never construct an open-redirect
|
||||
// payload (CWE-601). qa-loop iter-4 cluster
|
||||
// `users-page-null-map-and-open-redirect`.
|
||||
const rawNext = pathname + window.location.search
|
||||
const safeNext = sanitizeNextParam(rawNext)
|
||||
throw redirect({
|
||||
to: '/login',
|
||||
search: { next: pathname + window.location.search },
|
||||
search: safeNext ? { next: safeNext } : {},
|
||||
replace: true,
|
||||
})
|
||||
}
|
||||
@ -209,7 +215,12 @@ const loginRoute = createRoute({
|
||||
component: LoginPage,
|
||||
validateSearch: (raw: Record<string, unknown>): { next?: string; error?: string } => {
|
||||
const out: { next?: string; error?: string } = {}
|
||||
if (typeof raw.next === 'string' && raw.next.length > 0) out.next = raw.next
|
||||
// Sanitize `next` to prevent open-redirect (CWE-601): an attacker
|
||||
// could craft /login?next=//evil.com so post-login navigation
|
||||
// sends the operator off-origin. qa-loop iter-4 cluster
|
||||
// `users-page-null-map-and-open-redirect`.
|
||||
const safeNext = sanitizeNextParam(raw.next)
|
||||
if (safeNext) out.next = safeNext
|
||||
if (typeof raw.error === 'string' && raw.error.length > 0) out.error = raw.error
|
||||
return out
|
||||
},
|
||||
@ -237,7 +248,10 @@ const loginVerifyRoute = createRoute({
|
||||
if (typeof raw.requestId === 'string' && raw.requestId.length > 0) {
|
||||
out.requestId = raw.requestId
|
||||
}
|
||||
if (typeof raw.next === 'string' && raw.next.length > 0) out.next = raw.next
|
||||
// Same open-redirect sanitization as /login (CWE-601). qa-loop
|
||||
// iter-4 cluster `users-page-null-map-and-open-redirect`.
|
||||
const safeNext = sanitizeNextParam(raw.next)
|
||||
if (safeNext) out.next = safeNext
|
||||
return out
|
||||
},
|
||||
})
|
||||
|
||||
@ -81,6 +81,31 @@ describe('grantForScope', () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe('null-safety (qa-loop iter-4 cluster users-page-null-map-and-open-redirect)', () => {
|
||||
// The Go zero-value `[]User` slice serializes as `null`, not `[]`.
|
||||
// The matrix client mostly normalizes, but defense in depth here:
|
||||
// flattenForScope and grantForScope must not crash on a null users
|
||||
// array or a null per-user access map.
|
||||
it('flattenForScope tolerates users: null', () => {
|
||||
const broken = {
|
||||
users: null as unknown as never,
|
||||
applications: [],
|
||||
tiers: [],
|
||||
} as unknown as AccessMatrixResponse
|
||||
expect(flattenForScope(broken, { kind: 'application', value: 'wordpress' })).toEqual([])
|
||||
})
|
||||
|
||||
it('grantForScope tolerates user.access: null', () => {
|
||||
const user = {
|
||||
id: 'x',
|
||||
source: 'keycloak',
|
||||
access: null as unknown as never,
|
||||
}
|
||||
expect(grantForScope(user as never, { kind: 'application', value: 'wordpress' })).toBeUndefined()
|
||||
expect(grantForScope(user as never, { kind: 'organization', value: 'acme' })).toBeUndefined()
|
||||
})
|
||||
})
|
||||
|
||||
describe('defaultScopesForScope', () => {
|
||||
it('returns application scope for kind=application', () => {
|
||||
const out = defaultScopesForScope({ kind: 'application', value: 'wordpress' })
|
||||
|
||||
@ -23,13 +23,17 @@ export interface MemberRowData {
|
||||
|
||||
/** flattenForScope maps the access-matrix shape (user → application
|
||||
* → grant) to one row per user that has a grant on the requested
|
||||
* scope. Wildcards (`*`) and exact-app matches both qualify. */
|
||||
* scope. Wildcards (`*`) and exact-app matches both qualify.
|
||||
*
|
||||
* Defends against `users: null` from a Go zero-value slice — qa-loop
|
||||
* iter-4 cluster `users-page-null-map-and-open-redirect` (the
|
||||
* /users-page sibling crash). */
|
||||
export function flattenForScope(
|
||||
matrix: AccessMatrixResponse,
|
||||
scope: MembersScope,
|
||||
): MemberRowData[] {
|
||||
const out: MemberRowData[] = []
|
||||
for (const u of matrix.users) {
|
||||
for (const u of matrix.users ?? []) {
|
||||
const grant = grantForScope(u, scope)
|
||||
if (grant) out.push({ user: u, grant })
|
||||
}
|
||||
@ -41,15 +45,16 @@ export function grantForScope(
|
||||
user: AccessMatrixUser,
|
||||
scope: MembersScope,
|
||||
): AccessMatrixGrant | undefined {
|
||||
const access = user.access ?? {}
|
||||
if (scope.kind === 'application') {
|
||||
// Direct match on the application key, OR fallback to the
|
||||
// synthetic '*' (global) grant.
|
||||
return user.access[scope.value] ?? user.access['*']
|
||||
return access[scope.value] ?? access['*']
|
||||
}
|
||||
// For organization scope, the matrix already pre-filtered to the
|
||||
// org via the ?org=<slug> query — so any grant on the user is in
|
||||
// scope. Surface the highest-tier grant.
|
||||
const tiers = Object.values(user.access)
|
||||
const tiers = Object.values(access)
|
||||
return tiers[0]
|
||||
}
|
||||
|
||||
|
||||
@ -9,6 +9,7 @@
|
||||
|
||||
import { describe, it, expect, afterEach } from 'vitest'
|
||||
import { render, screen, cleanup } from '@testing-library/react'
|
||||
import { QueryClient, QueryClientProvider } from '@tanstack/react-query'
|
||||
import {
|
||||
RouterProvider,
|
||||
createRouter,
|
||||
@ -20,12 +21,17 @@ import {
|
||||
import { UserAccessListPage } from './UserAccessListPage'
|
||||
import type { UserAccessItem } from './userAccess.api'
|
||||
|
||||
function renderList(initialItems: UserAccessItem[]) {
|
||||
function renderList(initialItems: UserAccessItem[] | null | undefined) {
|
||||
const rootRoute = createRootRoute({ component: () => <Outlet /> })
|
||||
const listRoute = createRoute({
|
||||
getParentRoute: () => rootRoute,
|
||||
path: '/provision/$deploymentId/users',
|
||||
component: () => <UserAccessListPage initialItems={initialItems} disableFetch />,
|
||||
component: () => (
|
||||
<UserAccessListPage
|
||||
initialItems={initialItems as UserAccessItem[] | undefined}
|
||||
disableFetch
|
||||
/>
|
||||
),
|
||||
})
|
||||
const newRoute = createRoute({
|
||||
getParentRoute: () => rootRoute,
|
||||
@ -47,7 +53,18 @@ function renderList(initialItems: UserAccessItem[]) {
|
||||
routeTree: tree,
|
||||
history: createMemoryHistory({ initialEntries: ['/provision/d-1/users'] }),
|
||||
})
|
||||
return render(<RouterProvider router={router} />)
|
||||
// PortalShell + child sidebar consume useQuery hooks, so the tree
|
||||
// must be wrapped in a QueryClientProvider. Without it every test
|
||||
// bombs with "No QueryClient set, use QueryClientProvider to set one".
|
||||
// Fixed alongside qa-loop iter-4 cluster
|
||||
// `users-page-null-map-and-open-redirect` so the new null-safety
|
||||
// test rows can render at all.
|
||||
const qc = new QueryClient({ defaultOptions: { queries: { retry: false } } })
|
||||
return render(
|
||||
<QueryClientProvider client={qc}>
|
||||
<RouterProvider router={router} />
|
||||
</QueryClientProvider>,
|
||||
)
|
||||
}
|
||||
|
||||
afterEach(() => cleanup())
|
||||
@ -117,4 +134,36 @@ describe('UserAccessListPage', () => {
|
||||
expect(cta).toBeTruthy()
|
||||
expect(cta.getAttribute('href') || '').toContain('/provision/d-1/users/new')
|
||||
})
|
||||
|
||||
// qa-loop iter-4 cluster `users-page-null-map-and-open-redirect` —
|
||||
// TC-028/169/222 surfaced the page crashing with
|
||||
// `TypeError: Cannot read properties of null (reading 'map')` when
|
||||
// the BE serialized Go zero-value `[]Item` slices as `null` over
|
||||
// the wire. The page renders defensively now even if the API or a
|
||||
// future refactor leaks nulls back into the props.
|
||||
it('renders without crashing when item.spec.applications is null', async () => {
|
||||
const items = [
|
||||
{
|
||||
name: 'broken-grant',
|
||||
spec: {
|
||||
user: { keycloakSubject: 'eve' },
|
||||
sovereignRef: 'omantel',
|
||||
// Simulate a wire null leaking past the api-client normalizer.
|
||||
applications: null as unknown as never,
|
||||
},
|
||||
},
|
||||
] as unknown as UserAccessItem[]
|
||||
renderList(items)
|
||||
// The row renders — no React error boundary engaged.
|
||||
expect(await screen.findByTestId('user-access-row-broken-grant')).toBeTruthy()
|
||||
// The grants summary cell is empty, but the page is alive.
|
||||
expect(screen.getByText('eve')).toBeTruthy()
|
||||
})
|
||||
|
||||
it('renders without crashing when initialItems itself is null-ish', async () => {
|
||||
// Belt-and-suspenders: even if a parent component passes null/
|
||||
// undefined the page should render the empty-state without crashing.
|
||||
renderList(null as unknown as UserAccessItem[])
|
||||
expect(await screen.findByTestId('user-access-empty-state')).toBeTruthy()
|
||||
})
|
||||
})
|
||||
|
||||
@ -61,7 +61,10 @@ export function UserAccessListPage({
|
||||
listUserAccess(deploymentId)
|
||||
.then((rows) => {
|
||||
if (!cancelled) {
|
||||
setItems(rows)
|
||||
// Defense in depth — even if a future BE change returns
|
||||
// `null` for the items array, never crash. qa-loop iter-4
|
||||
// cluster `users-page-null-map-and-open-redirect`.
|
||||
setItems(rows ?? [])
|
||||
setError(null)
|
||||
}
|
||||
})
|
||||
@ -117,7 +120,7 @@ export function UserAccessListPage({
|
||||
|
||||
{loading ? (
|
||||
<div data-testid="user-access-loading" className="text-sm text-[var(--color-text-dim)]">Loading…</div>
|
||||
) : items.length === 0 ? (
|
||||
) : (items ?? []).length === 0 ? (
|
||||
<div
|
||||
data-testid="user-access-empty-state"
|
||||
className="rounded-md border border-[var(--color-border)] px-4 py-8 text-center text-sm text-[var(--color-text-dim)]"
|
||||
@ -138,7 +141,7 @@ export function UserAccessListPage({
|
||||
</tr>
|
||||
</thead>
|
||||
<tbody>
|
||||
{items.map((item) => (
|
||||
{(items ?? []).map((item) => (
|
||||
<tr
|
||||
key={item.name}
|
||||
data-testid={`user-access-row-${item.name}`}
|
||||
|
||||
@ -64,7 +64,38 @@ export async function listUserAccess(deploymentId: string): Promise<UserAccessIt
|
||||
throw new Error(`list user-access: HTTP ${res.status}`)
|
||||
}
|
||||
const body: UserAccessListResponse = await res.json()
|
||||
return body.items ?? []
|
||||
// Defensive normalization: a Go zero-value `[]Item` slice serializes
|
||||
// to `null`, not `[]`. Same for nested fields. The UI renders
|
||||
// `applications.map(...)` directly — null would crash the page with
|
||||
// `TypeError: Cannot read properties of null (reading 'map')`. Caught
|
||||
// on console.omantel.biz 2026-05-09 (qa-loop iter-4 cluster
|
||||
// `users-page-null-map-and-open-redirect`).
|
||||
const items = body.items ?? []
|
||||
return items.map(normalizeItem)
|
||||
}
|
||||
|
||||
/**
|
||||
* Defensively normalize a UserAccessItem so all array-shaped fields are
|
||||
* arrays (never null). Callers may render fields with `.map(...)` and
|
||||
* we never want a server-side null to crash the UI.
|
||||
*/
|
||||
function normalizeItem(item: UserAccessItem): UserAccessItem {
|
||||
return {
|
||||
...item,
|
||||
spec: {
|
||||
...item.spec,
|
||||
user: {
|
||||
keycloakSubject: item.spec?.user?.keycloakSubject,
|
||||
keycloakGroups: item.spec?.user?.keycloakGroups ?? undefined,
|
||||
},
|
||||
sovereignRef: item.spec?.sovereignRef ?? '',
|
||||
applications: (item.spec?.applications ?? []).map((g) => ({
|
||||
...g,
|
||||
namespaces: g.namespaces ?? undefined,
|
||||
vClusters: g.vClusters ?? undefined,
|
||||
})),
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
export async function createUserAccess(
|
||||
@ -126,7 +157,11 @@ export function userLabel(user: UserAccessUser): string {
|
||||
/**
|
||||
* Compact summary of an UserAccess Claim's grants for the list view:
|
||||
* "helmwatch (editor), catalyst (admin)"
|
||||
*
|
||||
* Defensively handles `null`/`undefined` because Go zero-value slices
|
||||
* serialize as `null` over the wire. qa-loop iter-4 cluster
|
||||
* `users-page-null-map-and-open-redirect`.
|
||||
*/
|
||||
export function grantsSummary(applications: UserAccessAppGrant[]): string {
|
||||
return applications.map((g) => `${g.app} (${g.role})`).join(', ')
|
||||
export function grantsSummary(applications: UserAccessAppGrant[] | null | undefined): string {
|
||||
return (applications ?? []).map((g) => `${g.app} (${g.role})`).join(', ')
|
||||
}
|
||||
|
||||
@ -19,6 +19,7 @@ import { AuthShell } from '@/app/layouts/AuthLayout'
|
||||
import { Button } from '@/shared/ui/button'
|
||||
import { PinInput6 } from '@/components/PinInput6'
|
||||
import { API_BASE } from '@/shared/config/urls'
|
||||
import { sanitizeNextParam } from '@/app/auth-gate'
|
||||
|
||||
type State = 'idle' | 'verifying' | 'error'
|
||||
|
||||
@ -92,7 +93,13 @@ export function VerifyPinPage() {
|
||||
// Hard navigation so the next page boot reads the cookie via
|
||||
// /whoami fresh and the auth gate sees the marker. Mirrors the
|
||||
// pattern Fix #A (PR #1093) uses on the unauth path.
|
||||
const target = next ?? '/wizard'
|
||||
//
|
||||
// Belt-and-suspenders open-redirect defense (CWE-601): even
|
||||
// though /login + /login/verify validateSearch already
|
||||
// sanitizes `next`, we sanitize again here in case a future
|
||||
// caller bypasses the route's validateSearch. qa-loop iter-4
|
||||
// cluster `users-page-null-map-and-open-redirect`.
|
||||
const target = sanitizeNextParam(next) ?? '/wizard'
|
||||
if (typeof window !== 'undefined') {
|
||||
window.location.replace(target)
|
||||
return
|
||||
|
||||
Loading…
Reference in New Issue
Block a user