Skip to content

Commit 2b30583

Browse files
os-zhuangclaude
os-zhuang
and
claude
authored
fix(permissions/fields): #2926 ④⑧ — FLS fail-open + lookup display_field (#2537)
* fix(permissions): authenticated fetch + authenticated-only fail-closed FLS (framework#2926 ④) /me/permissions was fetched with the cookie-only global fetch, so a token-only session (localStorage, no better-auth cookie) resolved as anonymous and FLS rendering fell open — restricted fields rendered editable and user input was silently stripped by the data layer. MePermissionsProvider now accepts a fetcher and the console injects createAuthenticatedFetch() (root cause), and the unknown-object default is authentication-gated: authenticated sessions fail closed on unconfigured objects, anonymous sessions keep the permissive default so guest/public forms keep working. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017bJWtKmZ2mFpRFAmmmoeqe * fix(fields): honor display_field in LookupCellRenderer (framework#2926 ⑧) ObjectGrid forwards display_field on the column meta, but the read cell ignored it and always ran the hardcoded name-first heuristics — lookup columns showed the raw API name instead of the configured display/label field. Thread the preferred field through every render path and the useLookupName fetch, keying the name cache by display field so two columns over the same record never serve each other's cached name. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017bJWtKmZ2mFpRFAmmmoeqe --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent cee5d6e commit 2b30583

7 files changed

Lines changed: 241 additions & 13 deletions

File tree

‎.changeset/fields-2926-lookup-display-field.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@object-ui/fields': patch
3+
---
4+
5+
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).

‎.changeset/fls-2926-authenticated-fail-closed.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
'@object-ui/permissions': minor
3+
'@object-ui/console': patch
4+
---
5+
6+
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.

‎apps/console/src/AppContent.tsx‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import { lazy, Suspense, useMemo } from 'react';
1212
import { Route, useParams, useLocation, Navigate } from 'react-router-dom';
1313
import { DefaultAppContent, LoadingScreen } from '@object-ui/app-shell';
1414
import { MePermissionsProvider } from '@object-ui/permissions';
15+
import { createAuthenticatedFetch } from '@object-ui/auth';
1516
import { LocalizationFetchProvider } from './LocalizationFetchProvider';
1617
import {
1718
UploadProvider,
@@ -125,8 +126,13 @@ export function AppContent() {
125126
() => createObjectStackUploadAdapter({ baseUrl: serverUrl }),
126127
[serverUrl],
127128
);
129+
// [#2926 ④] /me/permissions must carry the Bearer token like every other
130+
// data call (same wrapper AdapterProvider uses) — with the cookie-only
131+
// default fetch, a token-only session resolved as anonymous and FLS
132+
// rendering fell open (restricted fields rendered editable).
133+
const authFetch = useMemo(() => createAuthenticatedFetch(), []);
128134
return (
129-
<MePermissionsProvider endpoint={endpoint} loadingFallback={<LoadingScreen />}>
135+
<MePermissionsProvider endpoint={endpoint} fetcher={authFetch} loadingFallback={<LoadingScreen />}>
130136
<LocalizationFetchProvider endpoint={localizationEndpoint}>
131137
<UploadProvider adapter={uploadAdapter}>
132138
<DefaultAppContent extraRoutes={systemRoutes} extraRoutesNoApp={systemRoutes} />

‎packages/fields/src/__tests__/lookupCellDisplayField.test.tsx‎

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
/**
2+
* [framework#2926 ⑧] LookupCellRenderer must honor the target object's
3+
* configured display field. ObjectGrid forwards `display_field` on the
4+
* column meta (RELATIONAL_META_KEYS) exactly like `reference`, but the read
5+
* cell used to ignore it and always ran the hardcoded heuristics — `name`
6+
* first — so a target object whose displayNameField is a localized/label
7+
* field still rendered the raw API `name`.
8+
*/
9+
import { describe, it, expect, vi } from 'vitest';
10+
import { render, screen, waitFor } from '@testing-library/react';
11+
import '@testing-library/jest-dom';
12+
import React from 'react';
13+
14+
import { LookupCellRenderer } from '../index';
15+
import { SchemaRendererProvider } from '@object-ui/react';
16+
17+
const ID_A = '01ARZ3NDEKTSV4RRFFQ69G5FAA';
18+
const ID_B = '01ARZ3NDEKTSV4RRFFQ69G5FAB';
19+
20+
function makeDataSource() {
21+
const findOne = vi.fn(async (object: string, id: string) => {
22+
if (object === 'showcase_category') {
23+
// Record carries BOTH the raw API name and a localized label field.
24+
if (id === ID_A) return { id, name: 'cat_hardware', label_zh: '硬件' };
25+
if (id === ID_B) return { id, name: 'cat_software', label_zh: '软件' };
26+
}
27+
return null;
28+
});
29+
return { findOne, find: vi.fn() } as any;
30+
}
31+
32+
describe('LookupCellRenderer — display_field resolution', () => {
33+
it('prefers the configured display_field over the heuristic `name`', async () => {
34+
const ds = makeDataSource();
35+
render(
36+
<SchemaRendererProvider dataSource={ds}>
37+
<LookupCellRenderer
38+
value={ID_A}
39+
field={{ type: 'lookup', reference: 'showcase_category', display_field: 'label_zh' } as any}
40+
/>
41+
</SchemaRendererProvider>,
42+
);
43+
await waitFor(() => {
44+
expect(screen.getByText('硬件')).toBeInTheDocument();
45+
});
46+
expect(screen.queryByText('cat_hardware')).not.toBeInTheDocument();
47+
});
48+
49+
it('keeps the heuristic (`name` first) when no display_field is configured', async () => {
50+
const ds = makeDataSource();
51+
render(
52+
<SchemaRendererProvider dataSource={ds}>
53+
<LookupCellRenderer
54+
value={ID_A}
55+
field={{ type: 'lookup', reference: 'showcase_category' } as any}
56+
/>
57+
</SchemaRendererProvider>,
58+
);
59+
await waitFor(() => {
60+
expect(screen.getByText('cat_hardware')).toBeInTheDocument();
61+
});
62+
});
63+
64+
it('uses display_field on server-expanded nested objects (no fetch path)', () => {
65+
const ds = makeDataSource();
66+
render(
67+
<SchemaRendererProvider dataSource={ds}>
68+
<LookupCellRenderer
69+
value={{ id: ID_A, name: 'cat_hardware', label_zh: '硬件' } as any}
70+
field={{ type: 'lookup', reference: 'showcase_category', display_field: 'label_zh' } as any}
71+
/>
72+
</SchemaRendererProvider>,
73+
);
74+
expect(screen.getByText('硬件')).toBeInTheDocument();
75+
expect(ds.findOne).not.toHaveBeenCalled();
76+
});
77+
78+
it('does not serve a cached heuristic name to a display_field column (cache key isolation)', async () => {
79+
const ds = makeDataSource();
80+
// First: a column WITHOUT display_field resolves and caches the heuristic name.
81+
const first = render(
82+
<SchemaRendererProvider dataSource={ds}>
83+
<LookupCellRenderer
84+
value={ID_B}
85+
field={{ type: 'lookup', reference: 'showcase_category' } as any}
86+
/>
87+
</SchemaRendererProvider>,
88+
);
89+
await waitFor(() => {
90+
expect(screen.getByText('cat_software')).toBeInTheDocument();
91+
});
92+
first.unmount();
93+
// Then: a column WITH display_field for the same record must show the
94+
// configured field, not the previously cached heuristic name.
95+
render(
96+
<SchemaRendererProvider dataSource={ds}>
97+
<LookupCellRenderer
98+
value={ID_B}
99+
field={{ type: 'lookup', reference: 'showcase_category', display_field: 'label_zh' } as any}
100+
/>
101+
</SchemaRendererProvider>,
102+
);
103+
await waitFor(() => {
104+
expect(screen.getByText('软件')).toBeInTheDocument();
105+
});
106+
});
107+
});

‎packages/fields/src/index.tsx‎

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ export function isLikelyOpaqueId(v: unknown): boolean {
101101
* the context isn't installed. Returns the resolved display name or
102102
* `undefined` while pending or unresolvable.
103103
*/
104-
function useLookupName(referenceTo: string | undefined, value: unknown): string | undefined {
104+
function useLookupName(referenceTo: string | undefined, value: unknown, preferredField?: string): string | undefined {
105105
const ctx = React.useContext(_SchemaRendererContext);
106106
const dataSource = ctx?.dataSource;
107107
const [, force] = React.useState(0);
@@ -112,7 +112,10 @@ function useLookupName(referenceTo: string | undefined, value: unknown): string
112112
typeof dataSource.find === 'function' &&
113113
(typeof value === 'string' || typeof value === 'number') &&
114114
value !== '';
115-
const cacheKey = isResolvable ? `${referenceTo}:${String(value)}` : '';
115+
// The preferred display field is part of the cache identity: two columns
116+
// targeting the same record with different `display_field`s must not
117+
// serve each other's cached name (#2926 ⑧).
118+
const cacheKey = isResolvable ? `${referenceTo}:${String(value)}:${preferredField ?? ''}` : '';
116119

117120
React.useEffect(() => {
118121
if (!isResolvable) return;
@@ -135,7 +138,7 @@ function useLookupName(referenceTo: string | undefined, value: unknown): string
135138
: (result?.value || result?.data || []);
136139
record = records[0];
137140
}
138-
const name = pickRecordDisplayName(record);
141+
const name = pickRecordDisplayName(record, preferredField);
139142
lookupNameCache.set(cacheKey, { state: 'ok', name });
140143
} catch {
141144
lookupNameCache.set(cacheKey, { state: 'err' });
@@ -1178,6 +1181,13 @@ export function LookupCellRenderer({ value, field }: CellRendererProps): React.R
11781181
(field as { reference_to?: string }).reference_to ||
11791182
(field as { reference?: string }).reference;
11801183

1184+
// [#2926 ⑧] Honor the target object's configured display field. ObjectGrid
1185+
// forwards `display_field` on the column meta (RELATIONAL_META_KEYS) the
1186+
// same way it forwards `reference`; without threading it into
1187+
// pickRecordDisplayName the cell always fell back to the hardcoded
1188+
// heuristics (`name` first) and ignored displayNameField configuration.
1189+
const displayField = (field as { display_field?: string }).display_field;
1190+
11811191
// Pick the FIRST primitive id we see (for arrays, only the first one is auto-resolved
11821192
// to keep the cell cheap; multi-value lookups should generally be expanded server-side).
11831193
const primaryPrimitiveId = (() => {
@@ -1199,7 +1209,7 @@ export function LookupCellRenderer({ value, field }: CellRendererProps): React.R
11991209
})();
12001210

12011211
// Always call the hook (rules of hooks). It safely no-ops when inputs are missing.
1202-
const resolvedName = useLookupName(referenceTo, primaryPrimitiveId);
1212+
const resolvedName = useLookupName(referenceTo, primaryPrimitiveId, displayField);
12031213

12041214
if (value == null || value === '') return <EmptyValue />;
12051215

@@ -1213,7 +1223,7 @@ export function LookupCellRenderer({ value, field }: CellRendererProps): React.R
12131223
const parsed = JSON.parse(s) as Record<string, unknown>;
12141224
if (parsed && typeof parsed === 'object') {
12151225
const display =
1216-
pickRecordDisplayName(parsed) ||
1226+
pickRecordDisplayName(parsed, displayField) ||
12171227
String(parsed.externalId ?? parsed.id ?? parsed._id ?? '');
12181228
if (display) return <span className="truncate">{display}</span>;
12191229
}
@@ -1225,7 +1235,7 @@ export function LookupCellRenderer({ value, field }: CellRendererProps): React.R
12251235
// (e.g. { id, name }). Render its display name directly — no fetch needed.
12261236
if (!Array.isArray(value) && typeof value === 'object') {
12271237
const obj = value as Record<string, unknown>;
1228-
const display = pickRecordDisplayName(obj) || String(obj.id || obj._id || '');
1238+
const display = pickRecordDisplayName(obj, displayField) || String(obj.id || obj._id || '');
12291239
if (display) {
12301240
return <span className="truncate">{display}</span>;
12311241
}
@@ -1259,7 +1269,7 @@ export function LookupCellRenderer({ value, field }: CellRendererProps): React.R
12591269
let label: string;
12601270
let muted = false;
12611271
if (item != null && typeof item === 'object') {
1262-
label = pickRecordDisplayName(item as Record<string, unknown>) || String((item as any).id || (item as any)._id || '[Object]');
1272+
label = pickRecordDisplayName(item as Record<string, unknown>, displayField) || String((item as any).id || (item as any)._id || '[Object]');
12631273
} else {
12641274
const r = resolveLabel(item);
12651275
label = r.text;
@@ -1285,7 +1295,7 @@ export function LookupCellRenderer({ value, field }: CellRendererProps): React.R
12851295

12861296
if (typeof value === 'object' && value !== null) {
12871297
const label =
1288-
pickRecordDisplayName(value as Record<string, unknown>) ||
1298+
pickRecordDisplayName(value as Record<string, unknown>, displayField) ||
12891299
String((value as any).id || (value as any)._id || '[Object]');
12901300
return <span className="truncate">{label}</span>;
12911301
}

‎packages/permissions/src/MePermissionsProvider.tsx‎

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,15 @@ export interface MePermissionsResponse {
4343
export interface MePermissionsProviderProps {
4444
/** Absolute or relative URL to the /me/permissions endpoint */
4545
endpoint?: string;
46+
/**
47+
* Fetch implementation used to call the endpoint. Pass an authenticated
48+
* fetch (e.g. `createAuthenticatedFetch()` from `@object-ui/auth`) so the
49+
* request carries the Bearer token: with the default global `fetch` the
50+
* request is cookie-only, and a token-only session (localStorage, no
51+
* better-auth cookie) resolves as anonymous — the UI then renders
52+
* restricted fields as editable (#2926 ④).
53+
*/
54+
fetcher?: typeof fetch;
4655
/** Pre-fetched permissions payload (testing / SSR) */
4756
initialPermissions?: MePermissionsResponse;
4857
/** Rendered while permissions load (fail-closed) */
@@ -68,6 +77,7 @@ const DEFAULT_ENDPOINT = '/api/v1/auth/me/permissions';
6877
*/
6978
export function MePermissionsProvider({
7079
endpoint = DEFAULT_ENDPOINT,
80+
fetcher,
7181
initialPermissions,
7282
loadingFallback = null,
7383
errorFallback,
@@ -81,7 +91,8 @@ export function MePermissionsProvider({
8191
setLoading(true);
8292
setError(null);
8393
try {
84-
const res = await fetch(endpoint, {
94+
const doFetch = fetcher ?? fetch;
95+
const res = await doFetch(endpoint, {
8596
credentials: 'include',
8697
headers: { Accept: 'application/json' },
8798
});
@@ -93,7 +104,7 @@ export function MePermissionsProvider({
93104
} finally {
94105
setLoading(false);
95106
}
96-
}, [endpoint]);
107+
}, [endpoint, fetcher]);
97108

98109
useEffect(() => {
99110
if (initialPermissions) return;
@@ -115,7 +126,18 @@ export function MePermissionsProvider({
115126
}
116127
// No explicit field-level override → defer to object-level perms.
117128
const objPerm = data.objects?.[objKey] ?? data.objects?.[object] ?? data.objects?.['*'];
118-
if (!objPerm) return true; // permissive default when nothing configured
129+
if (!objPerm) {
130+
// [#2926 ④] Unknown-object default is authentication-gated:
131+
// - authenticated session → fail-CLOSED. The server resolved this
132+
// user's permissions and said nothing about the object, so
133+
// rendering it editable invites input the data layer will strip.
134+
// - anonymous (`authenticated: false` — the endpoint's no-session
135+
// 200 carries no objects/fields at all) → keep the permissive
136+
// default. Guest/public surfaces have no resolvable perms by
137+
// design; the server still enforces, and locking every field
138+
// would brick public forms.
139+
return data.authenticated !== true;
140+
}
119141
return action === 'read'
120142
? objPerm.allowRead !== false
121143
: objPerm.allowEdit !== false;
@@ -136,7 +158,8 @@ export function MePermissionsProvider({
136158
delete: 'allowDelete',
137159
};
138160
const k = map[action as string] ?? 'allowRead';
139-
const allowed = objPerm ? (objPerm as any)[k] !== false : true;
161+
// Same authentication-gated default as checkField (#2926 ④).
162+
const allowed = objPerm ? (objPerm as any)[k] !== false : data.authenticated !== true;
140163
return { allowed, reason: allowed ? undefined : 'denied-by-permission-set' };
141164
},
142165
[data],

‎packages/permissions/src/__tests__/MePermissionsProvider.test.tsx‎

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,77 @@ describe('MePermissionsProvider', () => {
9999
expect(screen.queryByTestId('read')).toBeNull();
100100
});
101101

102+
// [#2926 ④] Unknown-object default is authentication-gated.
103+
it('fails CLOSED for an authenticated user when the object has no configured perms', async () => {
104+
(global.fetch as any).mockResolvedValueOnce({
105+
ok: true,
106+
json: async () => ({
107+
authenticated: true,
108+
userId: 'u1',
109+
tenantId: 't1',
110+
roles: ['member'],
111+
permissionSets: ['restricted'],
112+
objects: { account: { allowRead: true, allowEdit: true } }, // no '*', nothing for 'project'
113+
fields: {},
114+
}),
115+
});
116+
117+
render(
118+
<MePermissionsProvider endpoint="/x">
119+
<Probe object="project" field="budget" />
120+
</MePermissionsProvider>,
121+
);
122+
123+
await waitFor(() => expect(screen.getByTestId('loaded').textContent).toBe('true'));
124+
expect(screen.getByTestId('read').textContent).toBe('false');
125+
expect(screen.getByTestId('write').textContent).toBe('false');
126+
});
127+
128+
it('keeps the permissive default for anonymous sessions (guest/public surfaces)', async () => {
129+
// The endpoint's no-session response: authenticated:false, NO objects/fields.
130+
(global.fetch as any).mockResolvedValueOnce({
131+
ok: true,
132+
json: async () => ({ authenticated: false }),
133+
});
134+
135+
render(
136+
<MePermissionsProvider endpoint="/x">
137+
<Probe object="showcase_inquiry" field="message" />
138+
</MePermissionsProvider>,
139+
);
140+
141+
await waitFor(() => expect(screen.getByTestId('loaded').textContent).toBe('true'));
142+
// Server still enforces; the anon UI must not brick public forms.
143+
expect(screen.getByTestId('read').textContent).toBe('true');
144+
expect(screen.getByTestId('write').textContent).toBe('true');
145+
});
146+
147+
it('uses the injected fetcher (authenticated fetch) instead of global fetch', async () => {
148+
const fetcher = vi.fn().mockResolvedValue({
149+
ok: true,
150+
json: async () => ({
151+
authenticated: true,
152+
userId: 'u1',
153+
tenantId: 't1',
154+
roles: [],
155+
permissionSets: [],
156+
objects: { '*': { allowRead: true, allowEdit: true } },
157+
fields: {},
158+
}),
159+
});
160+
161+
render(
162+
<MePermissionsProvider endpoint="/perm-endpoint" fetcher={fetcher as any}>
163+
<Probe object="account" field="name" />
164+
</MePermissionsProvider>,
165+
);
166+
167+
await waitFor(() => expect(screen.getByTestId('loaded').textContent).toBe('true'));
168+
expect(fetcher).toHaveBeenCalledWith('/perm-endpoint', expect.objectContaining({ credentials: 'include' }));
169+
expect(global.fetch).not.toHaveBeenCalled();
170+
expect(screen.getByTestId('write').textContent).toBe('true');
171+
});
172+
102173
it('skips fetch when initialPermissions provided', () => {
103174
render(
104175
<MePermissionsProvider

0 commit comments

Comments
 (0)