diff --git a/.changeset/fields-2926-lookup-display-field.md b/.changeset/fields-2926-lookup-display-field.md new file mode 100644 index 000000000..3aa164e95 --- /dev/null +++ b/.changeset/fields-2926-lookup-display-field.md @@ -0,0 +1,5 @@ +--- +'@object-ui/fields': patch +--- + +fix(fields): LookupCellRenderer honors the target object's configured `display_field` (framework#2926 ⑧). ObjectGrid already forwarded `display_field` on the column meta, but the read cell ignored it and always ran the hardcoded heuristics (`name` first), so lookup columns showed the raw API name instead of the configured display/label field. The preferred field now threads through every render path (expanded objects, arrays, JSON strings, and the on-demand `useLookupName` fetch, whose cache key includes the display field to prevent cross-column stale names). diff --git a/.changeset/fls-2926-authenticated-fail-closed.md b/.changeset/fls-2926-authenticated-fail-closed.md new file mode 100644 index 000000000..14c9b1d0d --- /dev/null +++ b/.changeset/fls-2926-authenticated-fail-closed.md @@ -0,0 +1,6 @@ +--- +'@object-ui/permissions': minor +'@object-ui/console': patch +--- + +fix(permissions): close the console FLS fail-open for token-only sessions (framework#2926 ④). Two halves: `MePermissionsProvider` gains a `fetcher` prop and the console passes `createAuthenticatedFetch()` so `/me/permissions` carries the Bearer token like every other data call (the cookie-only default fetch resolved token-only sessions as anonymous); and the unknown-object default is now authentication-gated — authenticated sessions fail CLOSED when an object has no resolved perms (fields render read-only instead of inviting input the data layer strips), while anonymous sessions keep the permissive default so guest/public forms keep working. Pairing note: with an older framework whose `/me/permissions` returns sparse objects for authenticated users, unconfigured objects now render read-only. diff --git a/apps/console/src/AppContent.tsx b/apps/console/src/AppContent.tsx index 84bcf0df5..d10739e3e 100644 --- a/apps/console/src/AppContent.tsx +++ b/apps/console/src/AppContent.tsx @@ -12,6 +12,7 @@ import { lazy, Suspense, useMemo } from 'react'; import { Route, useParams, useLocation, Navigate } from 'react-router-dom'; import { DefaultAppContent, LoadingScreen } from '@object-ui/app-shell'; import { MePermissionsProvider } from '@object-ui/permissions'; +import { createAuthenticatedFetch } from '@object-ui/auth'; import { LocalizationFetchProvider } from './LocalizationFetchProvider'; import { UploadProvider, @@ -125,8 +126,13 @@ export function AppContent() { () => createObjectStackUploadAdapter({ baseUrl: serverUrl }), [serverUrl], ); + // [#2926 ④] /me/permissions must carry the Bearer token like every other + // data call (same wrapper AdapterProvider uses) — with the cookie-only + // default fetch, a token-only session resolved as anonymous and FLS + // rendering fell open (restricted fields rendered editable). + const authFetch = useMemo(() => createAuthenticatedFetch(), []); return ( - }> + }> diff --git a/packages/fields/src/__tests__/lookupCellDisplayField.test.tsx b/packages/fields/src/__tests__/lookupCellDisplayField.test.tsx new file mode 100644 index 000000000..7d68c70cd --- /dev/null +++ b/packages/fields/src/__tests__/lookupCellDisplayField.test.tsx @@ -0,0 +1,107 @@ +/** + * [framework#2926 ⑧] LookupCellRenderer must honor the target object's + * configured display field. ObjectGrid forwards `display_field` on the + * column meta (RELATIONAL_META_KEYS) exactly like `reference`, but the read + * cell used to ignore it and always ran the hardcoded heuristics — `name` + * first — so a target object whose displayNameField is a localized/label + * field still rendered the raw API `name`. + */ +import { describe, it, expect, vi } from 'vitest'; +import { render, screen, waitFor } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import React from 'react'; + +import { LookupCellRenderer } from '../index'; +import { SchemaRendererProvider } from '@object-ui/react'; + +const ID_A = '01ARZ3NDEKTSV4RRFFQ69G5FAA'; +const ID_B = '01ARZ3NDEKTSV4RRFFQ69G5FAB'; + +function makeDataSource() { + const findOne = vi.fn(async (object: string, id: string) => { + if (object === 'showcase_category') { + // Record carries BOTH the raw API name and a localized label field. + if (id === ID_A) return { id, name: 'cat_hardware', label_zh: '硬件' }; + if (id === ID_B) return { id, name: 'cat_software', label_zh: '软件' }; + } + return null; + }); + return { findOne, find: vi.fn() } as any; +} + +describe('LookupCellRenderer — display_field resolution', () => { + it('prefers the configured display_field over the heuristic `name`', async () => { + const ds = makeDataSource(); + render( + + + , + ); + await waitFor(() => { + expect(screen.getByText('硬件')).toBeInTheDocument(); + }); + expect(screen.queryByText('cat_hardware')).not.toBeInTheDocument(); + }); + + it('keeps the heuristic (`name` first) when no display_field is configured', async () => { + const ds = makeDataSource(); + render( + + + , + ); + await waitFor(() => { + expect(screen.getByText('cat_hardware')).toBeInTheDocument(); + }); + }); + + it('uses display_field on server-expanded nested objects (no fetch path)', () => { + const ds = makeDataSource(); + render( + + + , + ); + expect(screen.getByText('硬件')).toBeInTheDocument(); + expect(ds.findOne).not.toHaveBeenCalled(); + }); + + it('does not serve a cached heuristic name to a display_field column (cache key isolation)', async () => { + const ds = makeDataSource(); + // First: a column WITHOUT display_field resolves and caches the heuristic name. + const first = render( + + + , + ); + await waitFor(() => { + expect(screen.getByText('cat_software')).toBeInTheDocument(); + }); + first.unmount(); + // Then: a column WITH display_field for the same record must show the + // configured field, not the previously cached heuristic name. + render( + + + , + ); + await waitFor(() => { + expect(screen.getByText('软件')).toBeInTheDocument(); + }); + }); +}); diff --git a/packages/fields/src/index.tsx b/packages/fields/src/index.tsx index d67b95b7a..f6e17a074 100644 --- a/packages/fields/src/index.tsx +++ b/packages/fields/src/index.tsx @@ -101,7 +101,7 @@ export function isLikelyOpaqueId(v: unknown): boolean { * the context isn't installed. Returns the resolved display name or * `undefined` while pending or unresolvable. */ -function useLookupName(referenceTo: string | undefined, value: unknown): string | undefined { +function useLookupName(referenceTo: string | undefined, value: unknown, preferredField?: string): string | undefined { const ctx = React.useContext(_SchemaRendererContext); const dataSource = ctx?.dataSource; const [, force] = React.useState(0); @@ -112,7 +112,10 @@ function useLookupName(referenceTo: string | undefined, value: unknown): string typeof dataSource.find === 'function' && (typeof value === 'string' || typeof value === 'number') && value !== ''; - const cacheKey = isResolvable ? `${referenceTo}:${String(value)}` : ''; + // The preferred display field is part of the cache identity: two columns + // targeting the same record with different `display_field`s must not + // serve each other's cached name (#2926 ⑧). + const cacheKey = isResolvable ? `${referenceTo}:${String(value)}:${preferredField ?? ''}` : ''; React.useEffect(() => { if (!isResolvable) return; @@ -135,7 +138,7 @@ function useLookupName(referenceTo: string | undefined, value: unknown): string : (result?.value || result?.data || []); record = records[0]; } - const name = pickRecordDisplayName(record); + const name = pickRecordDisplayName(record, preferredField); lookupNameCache.set(cacheKey, { state: 'ok', name }); } catch { lookupNameCache.set(cacheKey, { state: 'err' }); @@ -1178,6 +1181,13 @@ export function LookupCellRenderer({ value, field }: CellRendererProps): React.R (field as { reference_to?: string }).reference_to || (field as { reference?: string }).reference; + // [#2926 ⑧] Honor the target object's configured display field. ObjectGrid + // forwards `display_field` on the column meta (RELATIONAL_META_KEYS) the + // same way it forwards `reference`; without threading it into + // pickRecordDisplayName the cell always fell back to the hardcoded + // heuristics (`name` first) and ignored displayNameField configuration. + const displayField = (field as { display_field?: string }).display_field; + // Pick the FIRST primitive id we see (for arrays, only the first one is auto-resolved // to keep the cell cheap; multi-value lookups should generally be expanded server-side). const primaryPrimitiveId = (() => { @@ -1199,7 +1209,7 @@ export function LookupCellRenderer({ value, field }: CellRendererProps): React.R })(); // Always call the hook (rules of hooks). It safely no-ops when inputs are missing. - const resolvedName = useLookupName(referenceTo, primaryPrimitiveId); + const resolvedName = useLookupName(referenceTo, primaryPrimitiveId, displayField); if (value == null || value === '') return ; @@ -1213,7 +1223,7 @@ export function LookupCellRenderer({ value, field }: CellRendererProps): React.R const parsed = JSON.parse(s) as Record; if (parsed && typeof parsed === 'object') { const display = - pickRecordDisplayName(parsed) || + pickRecordDisplayName(parsed, displayField) || String(parsed.externalId ?? parsed.id ?? parsed._id ?? ''); if (display) return {display}; } @@ -1225,7 +1235,7 @@ export function LookupCellRenderer({ value, field }: CellRendererProps): React.R // (e.g. { id, name }). Render its display name directly — no fetch needed. if (!Array.isArray(value) && typeof value === 'object') { const obj = value as Record; - const display = pickRecordDisplayName(obj) || String(obj.id || obj._id || ''); + const display = pickRecordDisplayName(obj, displayField) || String(obj.id || obj._id || ''); if (display) { return {display}; } @@ -1259,7 +1269,7 @@ export function LookupCellRenderer({ value, field }: CellRendererProps): React.R let label: string; let muted = false; if (item != null && typeof item === 'object') { - label = pickRecordDisplayName(item as Record) || String((item as any).id || (item as any)._id || '[Object]'); + label = pickRecordDisplayName(item as Record, displayField) || String((item as any).id || (item as any)._id || '[Object]'); } else { const r = resolveLabel(item); label = r.text; @@ -1285,7 +1295,7 @@ export function LookupCellRenderer({ value, field }: CellRendererProps): React.R if (typeof value === 'object' && value !== null) { const label = - pickRecordDisplayName(value as Record) || + pickRecordDisplayName(value as Record, displayField) || String((value as any).id || (value as any)._id || '[Object]'); return {label}; } diff --git a/packages/permissions/src/MePermissionsProvider.tsx b/packages/permissions/src/MePermissionsProvider.tsx index 5ce2c906b..dbd7240ff 100644 --- a/packages/permissions/src/MePermissionsProvider.tsx +++ b/packages/permissions/src/MePermissionsProvider.tsx @@ -43,6 +43,15 @@ export interface MePermissionsResponse { export interface MePermissionsProviderProps { /** Absolute or relative URL to the /me/permissions endpoint */ endpoint?: string; + /** + * Fetch implementation used to call the endpoint. Pass an authenticated + * fetch (e.g. `createAuthenticatedFetch()` from `@object-ui/auth`) so the + * request carries the Bearer token: with the default global `fetch` the + * request is cookie-only, and a token-only session (localStorage, no + * better-auth cookie) resolves as anonymous — the UI then renders + * restricted fields as editable (#2926 ④). + */ + fetcher?: typeof fetch; /** Pre-fetched permissions payload (testing / SSR) */ initialPermissions?: MePermissionsResponse; /** Rendered while permissions load (fail-closed) */ @@ -68,6 +77,7 @@ const DEFAULT_ENDPOINT = '/api/v1/auth/me/permissions'; */ export function MePermissionsProvider({ endpoint = DEFAULT_ENDPOINT, + fetcher, initialPermissions, loadingFallback = null, errorFallback, @@ -81,7 +91,8 @@ export function MePermissionsProvider({ setLoading(true); setError(null); try { - const res = await fetch(endpoint, { + const doFetch = fetcher ?? fetch; + const res = await doFetch(endpoint, { credentials: 'include', headers: { Accept: 'application/json' }, }); @@ -93,7 +104,7 @@ export function MePermissionsProvider({ } finally { setLoading(false); } - }, [endpoint]); + }, [endpoint, fetcher]); useEffect(() => { if (initialPermissions) return; @@ -115,7 +126,18 @@ export function MePermissionsProvider({ } // No explicit field-level override → defer to object-level perms. const objPerm = data.objects?.[objKey] ?? data.objects?.[object] ?? data.objects?.['*']; - if (!objPerm) return true; // permissive default when nothing configured + if (!objPerm) { + // [#2926 ④] Unknown-object default is authentication-gated: + // - authenticated session → fail-CLOSED. The server resolved this + // user's permissions and said nothing about the object, so + // rendering it editable invites input the data layer will strip. + // - anonymous (`authenticated: false` — the endpoint's no-session + // 200 carries no objects/fields at all) → keep the permissive + // default. Guest/public surfaces have no resolvable perms by + // design; the server still enforces, and locking every field + // would brick public forms. + return data.authenticated !== true; + } return action === 'read' ? objPerm.allowRead !== false : objPerm.allowEdit !== false; @@ -136,7 +158,8 @@ export function MePermissionsProvider({ delete: 'allowDelete', }; const k = map[action as string] ?? 'allowRead'; - const allowed = objPerm ? (objPerm as any)[k] !== false : true; + // Same authentication-gated default as checkField (#2926 ④). + const allowed = objPerm ? (objPerm as any)[k] !== false : data.authenticated !== true; return { allowed, reason: allowed ? undefined : 'denied-by-permission-set' }; }, [data], diff --git a/packages/permissions/src/__tests__/MePermissionsProvider.test.tsx b/packages/permissions/src/__tests__/MePermissionsProvider.test.tsx index 3d6f6e67d..7bb13999a 100644 --- a/packages/permissions/src/__tests__/MePermissionsProvider.test.tsx +++ b/packages/permissions/src/__tests__/MePermissionsProvider.test.tsx @@ -99,6 +99,77 @@ describe('MePermissionsProvider', () => { expect(screen.queryByTestId('read')).toBeNull(); }); + // [#2926 ④] Unknown-object default is authentication-gated. + it('fails CLOSED for an authenticated user when the object has no configured perms', async () => { + (global.fetch as any).mockResolvedValueOnce({ + ok: true, + json: async () => ({ + authenticated: true, + userId: 'u1', + tenantId: 't1', + roles: ['member'], + permissionSets: ['restricted'], + objects: { account: { allowRead: true, allowEdit: true } }, // no '*', nothing for 'project' + fields: {}, + }), + }); + + render( + + + , + ); + + await waitFor(() => expect(screen.getByTestId('loaded').textContent).toBe('true')); + expect(screen.getByTestId('read').textContent).toBe('false'); + expect(screen.getByTestId('write').textContent).toBe('false'); + }); + + it('keeps the permissive default for anonymous sessions (guest/public surfaces)', async () => { + // The endpoint's no-session response: authenticated:false, NO objects/fields. + (global.fetch as any).mockResolvedValueOnce({ + ok: true, + json: async () => ({ authenticated: false }), + }); + + render( + + + , + ); + + await waitFor(() => expect(screen.getByTestId('loaded').textContent).toBe('true')); + // Server still enforces; the anon UI must not brick public forms. + expect(screen.getByTestId('read').textContent).toBe('true'); + expect(screen.getByTestId('write').textContent).toBe('true'); + }); + + it('uses the injected fetcher (authenticated fetch) instead of global fetch', async () => { + const fetcher = vi.fn().mockResolvedValue({ + ok: true, + json: async () => ({ + authenticated: true, + userId: 'u1', + tenantId: 't1', + roles: [], + permissionSets: [], + objects: { '*': { allowRead: true, allowEdit: true } }, + fields: {}, + }), + }); + + render( + + + , + ); + + await waitFor(() => expect(screen.getByTestId('loaded').textContent).toBe('true')); + expect(fetcher).toHaveBeenCalledWith('/perm-endpoint', expect.objectContaining({ credentials: 'include' })); + expect(global.fetch).not.toHaveBeenCalled(); + expect(screen.getByTestId('write').textContent).toBe('true'); + }); + it('skips fetch when initialPermissions provided', () => { render(