diff --git a/.changeset/12103-field-write-grants.md b/.changeset/12103-field-write-grants.md new file mode 100644 index 0000000000..ba504f6a66 --- /dev/null +++ b/.changeset/12103-field-write-grants.md @@ -0,0 +1,21 @@ +--- +'@object-ui/core': minor +'@object-ui/permissions': minor +'@object-ui/plugin-detail': patch +'@object-ui/plugin-grid': patch +--- + +In-place edit on the record page and a list's in-cell edit honour the caller's field write grant, and the owner field honours the transfer grant (objectui#12103). + +**What a caller saw.** A caller whose permission set marks a field `{ readable: true, editable: false }` was offered an editor for it in the record page's details body, in its highlights strip and in a list's grid cells, and Save answered 403 ("Field write denied"). The owner field was offered the same way to a caller without the transfer grant, and Save answered 403 ("requires the transfer grant"). Edit forms already refused both fields through `fieldWriteGate`. + +**What changed.** + +- **The map (`@object-ui/core`).** The `recordEdit` and `listInlineEdit` rows of `AFFORDANCE_GRANTS` gain `field: 'write'`, so the record page's in-place edit and a list's in-cell edit each ask every field the same update question an edit form asks, through `resolveFieldAffordance`. `FieldAffordance` therefore includes both rows. +- **The details body and the highlights strip (`@object-ui/plugin-detail`).** `DetailSection` and `HeaderHighlight` ask that question of each field. A refused field shows no pencil, does not enter the edit session on double-click, and stays a read display in edit mode. The strip and the body share one session and one Save, so they answer alike. +- **The grid (`@object-ui/plugin-grid`).** `ObjectGrid` marks a refused field's column not editable, so its cell keeps the read display while the rest of the row edits. Whether the grid is editable at all is still the object-level `listInlineEdit` verdict. +- **The owner field (`@object-ui/permissions`).** The server refuses an update that writes the record's owner (the field the `@objectstack/spec` constant `SystemFieldName` names `OWNER_ID`) unless the caller holds `allowTransfer` or `modifyAllRecords`. `MePermissionsProvider`'s `checkField(object, owner, 'write')` now answers with that grant, read with the spec's `objectPermissionGrants`. The `write` question is the server's update rule for a field, so every surface that asks it (edit forms, in-place edit, in-cell edit) offers the owner field only to a caller who may transfer the record. `checkField(object, owner, 'create')` is unchanged, because on insert the server stamps an empty owner to the caller and accepts the caller's own id. `MePermissionsResponse` declares the `allowTransfer` bit the endpoint already serves, and `@object-ui/permissions` now depends on `@objectstack/spec` directly. + +**Unchanged.** With no permission provider mounted no field question is asked, so a standalone embed and the designer preview behave as before. The server still enforces every grant; this only stops the console from offering a write it would refuse. + +**Clause-②: yes (widening).** `AFFORDANCE_GRANTS.recordEdit` and `AFFORDANCE_GRANTS.listInlineEdit` gain a `field: 'write'` member, which widens the `FieldAffordance` type by those two rows. `MePermissionsResponse`'s object entry declares an optional `allowTransfer?: boolean`. `checkField`'s action union is unchanged; its `write` answer narrows for the owner field only. No export, prop or language-pack key is added or removed. diff --git a/packages/core/README.md b/packages/core/README.md index 52ee53655c..e9c2a1613a 100644 --- a/packages/core/README.md +++ b/packages/core/README.md @@ -187,7 +187,13 @@ row's Delete, and the rest — reads the grant it exercises from ONE table, `AFFORDANCE_GRANTS` (objectui#12082). A row names the CRUD-affordance bit the affordance needs, the object grant it exercises (`create`, `update` or `delete`) and, for an affordance that offers fields, the field question it asks -(`create` for an insert, `write` for an update). +(`create` for an insert, `write` for an update). The fields of a form, of the +record page's in-place edit (`recordEdit`) and of a list's in-cell edit +(`listInlineEdit`) each ask that question, so a field the caller's permission +set marks `editable: false` is offered read-only on every one of them +(objectui#12103). The `write` question is the server's whole update rule for a +field, so for the record's owner field it also needs the transfer grant +(`allowTransfer`, or `modifyAllRecords`). `resolveAffordance` is the one verdict: the object's managed-object policy, the server's effective API operation set and the caller's grant must all allow it, diff --git a/packages/core/src/utils/affordanceGrants.ts b/packages/core/src/utils/affordanceGrants.ts index b0fb841c7a..2b93624b18 100644 --- a/packages/core/src/utils/affordanceGrants.ts +++ b/packages/core/src/utils/affordanceGrants.ts @@ -47,6 +47,16 @@ * way for both questions. So a create form follows what the server enforces on * insert, and adds no rule the server does not have. * + * The `write` question is the server's whole update rule for the field, so it + * also carries the one field whose update needs an OBJECT grant beyond + * `allowEdit`: the record's owner, the field the spec's `SystemFieldName` + * constant names `OWNER_ID`. The server refuses an update that writes it without + * the transfer grant (`allowTransfer`, or `modifyAllRecords`), so the resolver + * answers `write` for it with that grant too (objectui#12103). Every row that + * asks `write` therefore offers the owner field only to a caller who may + * transfer the record — the in-place edit, the list's in-cell edit and an edit + * form alike, with no row of its own to forget. + * * ## The verdict * * An affordance shows when the managed-object policy, the effective API @@ -120,8 +130,12 @@ export const AFFORDANCE_GRANTS = { createFormFields: { crud: 'create', grant: 'create', field: 'create' }, /** An edit form's fields (every `ObjectForm` layout) and its form-wide lock. */ editFormFields: { crud: 'edit', grant: 'update', field: 'write' }, - /** The record page's Edit — the header CTA and the record body's in-place editing. */ - recordEdit: { crud: 'edit', grant: 'update' }, + /** + * The record page's Edit — the header CTA and the record body's in-place + * editing (the details body and the highlights strip), each of whose fields + * asks the update question (objectui#12103). + */ + recordEdit: { crud: 'edit', grant: 'update', field: 'write' }, /** The record page's Delete. */ recordDelete: { crud: 'delete', grant: 'delete' }, /** @@ -133,8 +147,8 @@ export const AFFORDANCE_GRANTS = { listImport: { crud: 'import', grant: 'create', field: 'create' }, /** The import wizard's template download — its endpoint answers 403 without the create grant. */ importTemplate: { crud: null, grant: 'create' }, - /** A list's inline (in-cell) editing. */ - listInlineEdit: { crud: 'edit', grant: 'update' }, + /** A list's inline (in-cell) editing, each of whose cells asks the update question (objectui#12103). */ + listInlineEdit: { crud: 'edit', grant: 'update', field: 'write' }, /** A list's bulk Delete. */ listBulkDelete: { crud: 'delete', grant: 'delete' }, /** A grid row's Edit. */ diff --git a/packages/permissions/README.md b/packages/permissions/README.md index 33cf62967a..fa5956420f 100644 --- a/packages/permissions/README.md +++ b/packages/permissions/README.md @@ -148,6 +148,14 @@ such a field through and object admission decides, so `'write'` falls back to the object's `allowEdit` and `'create'` to its `allowCreate`. An explicit field-level entry answers both the same way. +One field's update needs more than its own grant: the record's owner, the +field the spec's `SystemFieldName` constant names `OWNER_ID`. The server refuses an +update that writes it unless the caller holds the transfer grant +(`allowTransfer`, or `modifyAllRecords`), so `MePermissionsProvider` answers +`'write'` for the owner field with that grant as well (objectui#12103). +`'create'` is unchanged: on insert the server stamps an empty owner to the +caller and accepts the caller's own id. + ```tsx import { usePermissions } from '@object-ui/permissions'; diff --git a/packages/permissions/package.json b/packages/permissions/package.json index 77887fb437..e7a44d0e85 100644 --- a/packages/permissions/package.json +++ b/packages/permissions/package.json @@ -37,7 +37,8 @@ "react": "^18.0.0 || ^19.0.0" }, "dependencies": { - "@object-ui/types": "workspace:*" + "@object-ui/types": "workspace:*", + "@objectstack/spec": "^17.7.0" }, "devDependencies": { "@types/react": "19.2.18", diff --git a/packages/permissions/src/MePermissionsProvider.tsx b/packages/permissions/src/MePermissionsProvider.tsx index 0fe29ed70d..9ab00a250c 100644 --- a/packages/permissions/src/MePermissionsProvider.tsx +++ b/packages/permissions/src/MePermissionsProvider.tsx @@ -19,6 +19,8 @@ import type { PermissionCheckResult, FieldLevelPermission, } from '@object-ui/types'; +import { objectPermissionGrants } from '@objectstack/spec/security'; +import { SystemFieldName } from '@objectstack/spec/system'; import { PermCtx, type PermissionContextValue } from './PermissionContext.js'; import { createDiscardProofCache } from './discardProofCache.js'; @@ -42,6 +44,13 @@ export interface MePermissionsResponse { allowDelete?: boolean; viewAllRecords?: boolean; modifyAllRecords?: boolean; + /** + * The transfer grant: may the caller change a record's owner. The server + * refuses an update that writes the owner field without it (or without + * `modifyAllRecords`, which implies it), and `checkField` reads it for that + * field's `write` question (objectui#12103). + */ + allowTransfer?: boolean; /** * [objectstack#3391] Server-resolved effective API operation set for this object * (enum-ordered). Present only when the object tightens exposure via @@ -307,6 +316,23 @@ export function MePermissionsProvider({ const objKey = (object ?? '').toLowerCase(); const key = `${objKey}.${field}`; const fieldPerm = data.fields?.[key] ?? data.fields?.[`${object}.${field}`]; + const objPerm = data.objects?.[objKey] ?? data.objects?.[object] ?? data.objects?.['*']; + // [objectui#12103] An update that writes the record's OWNER is a + // transfer: the server's security step refuses it unless the caller holds + // the transfer grant, whatever the field's own entry says. So `write` for + // that field needs the grant too, read with the spec's own predicate + // (`allowTransfer`, or `modifyAllRecords`, which implies it). The field is + // the one the spec names, as the server's guard keys it. `create` is not + // narrowed: on insert the server stamps an empty owner to the caller and + // accepts the caller's own id. An object the answer does not mention + // takes the unknown-object default below. + if ( + action === 'write' && + field === SystemFieldName.OWNER_ID && + !(objPerm ? objectPermissionGrants(objPerm, 'allowTransfer') : data.authenticated !== true) + ) { + return false; + } // An explicit entry answers `write` and `create` alike: the server's // field step refuses a non-editable field on insert and update both. if (fieldPerm) { @@ -318,7 +344,6 @@ export function MePermissionsProvider({ // field step lets the field through and object admission decides, so // the answer is the grant of the operation asked about (objectui#12082) // — `allowCreate` on an insert, `allowEdit` on an update. - const objPerm = data.objects?.[objKey] ?? data.objects?.[object] ?? data.objects?.['*']; if (!objPerm) { // [objectstack-ai/objectstack#2926 ④] Unknown-object default is authentication-gated: // - authenticated session → fail-CLOSED. The server resolved this diff --git a/packages/permissions/src/PermissionContext.ts b/packages/permissions/src/PermissionContext.ts index 9dff8b1f80..6be4789bef 100644 --- a/packages/permissions/src/PermissionContext.ts +++ b/packages/permissions/src/PermissionContext.ts @@ -21,6 +21,10 @@ export interface PermissionContextValue { * permission set does not mention: the server's field step lets it through * and object admission decides — `allowEdit` on an update, `allowCreate` on * an insert. An explicit field-level entry answers both the same way. + * + * `write` is the server's whole update rule, so for the record's owner field + * (`SystemFieldName.OWNER_ID`) it also needs the transfer grant — the server + * refuses an update that writes the owner without it (objectui#12103). */ checkField: (object: string, field: string, action: 'read' | 'write' | 'create') => boolean; /** Get field permissions for an object */ diff --git a/packages/permissions/src/__tests__/MePermissionsProvider.ownerTransfer-12103.test.tsx b/packages/permissions/src/__tests__/MePermissionsProvider.ownerTransfer-12103.test.tsx new file mode 100644 index 0000000000..da7e6bde9e --- /dev/null +++ b/packages/permissions/src/__tests__/MePermissionsProvider.ownerTransfer-12103.test.tsx @@ -0,0 +1,93 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * objectui#12103 — `checkField(object, owner, 'write')` answers the server's + * whole update rule for the owner field, which includes the transfer grant. + * + * The server's security step refuses an update that writes the record's owner + * (`SystemFieldName.OWNER_ID`) unless the caller holds `allowTransfer` or + * `modifyAllRecords`; measured on a live backend, `/me/permissions` reports the + * grant per object and the refusal is a 403. So the `write` question for that + * one field needs the grant, read with the spec's own predicate, while `create` + * (the insert rule, which stamps and accepts the caller's own id), `read`, and + * every other field are untouched. + */ + +import { describe, it, expect, afterEach } from 'vitest'; +import { render, cleanup } from '@testing-library/react'; +import React from 'react'; +import { SystemFieldName } from '@objectstack/spec/system'; +import { MePermissionsProvider, type MePermissionsResponse } from '../MePermissionsProvider'; +import { usePermissions } from '../usePermissions'; +import type { PermissionContextValue } from '../PermissionContext'; + +afterEach(cleanup); + +const OBJECT = 'crm_account'; +const OWNER = SystemFieldName.OWNER_ID; +const EDITOR = { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: false }; + +function envelope( + objects: MePermissionsResponse['objects'], + fields: MePermissionsResponse['fields'] = {}, + authenticated = true, +): MePermissionsResponse { + return { authenticated, userId: authenticated ? 'u1' : null, tenantId: null, roles: [], permissionSets: [], objects, fields }; +} + +function checkFieldUnder(perms: MePermissionsResponse): PermissionContextValue['checkField'] { + let captured: PermissionContextValue['checkField'] | null = null; + function Probe() { + captured = usePermissions().checkField; + return null; + } + render( + + + , + ); + if (!captured) throw new Error('the probe did not render'); + return captured; +} + +describe("checkField's write question for the owner field (objectui#12103)", () => { + it('is refused without the transfer grant — false and absent alike', () => { + expect(checkFieldUnder(envelope({ [OBJECT]: { ...EDITOR, allowTransfer: false } }))(OBJECT, OWNER, 'write')).toBe(false); + expect(checkFieldUnder(envelope({ [OBJECT]: { ...EDITOR } }))(OBJECT, OWNER, 'write')).toBe(false); + }); + + it('is granted by allowTransfer, and by modifyAllRecords, which implies it', () => { + expect(checkFieldUnder(envelope({ [OBJECT]: { ...EDITOR, allowTransfer: true } }))(OBJECT, OWNER, 'write')).toBe(true); + expect(checkFieldUnder(envelope({ [OBJECT]: { ...EDITOR, modifyAllRecords: true } }))(OBJECT, OWNER, 'write')).toBe(true); + }); + + it('reads the wildcard entry for an object the answer does not name, as the object fallback does', () => { + expect(checkFieldUnder(envelope({ '*': { ...EDITOR, modifyAllRecords: true } }))(OBJECT, OWNER, 'write')).toBe(true); + expect(checkFieldUnder(envelope({ '*': { ...EDITOR, allowTransfer: false } }))(OBJECT, OWNER, 'write')).toBe(false); + }); + + it('keeps the unknown-object default: closed for a signed-in caller, open for an anonymous one', () => { + expect(checkFieldUnder(envelope({}))(OBJECT, OWNER, 'write')).toBe(false); + expect(checkFieldUnder(envelope({}, {}, false))(OBJECT, OWNER, 'write')).toBe(true); + }); + + it("an explicit editable: false entry still refuses it with the grant; the grant does not open the field's own refusal", () => { + const check = checkFieldUnder( + envelope({ [OBJECT]: { ...EDITOR, allowTransfer: true } }, { [`${OBJECT}.${OWNER}`]: { readable: true, editable: false } }), + ); + expect(check(OBJECT, OWNER, 'write')).toBe(false); + }); + + it('CONTROL: create, read and every other field are untouched by the transfer grant', () => { + const check = checkFieldUnder(envelope({ [OBJECT]: { ...EDITOR, allowTransfer: false } })); + expect(check(OBJECT, OWNER, 'create')).toBe(true); + expect(check(OBJECT, OWNER, 'read')).toBe(true); + expect(check(OBJECT, 'name', 'write')).toBe(true); + }); +}); diff --git a/packages/plugin-detail/src/DetailSection.tsx b/packages/plugin-detail/src/DetailSection.tsx index cb928a73c3..c357d1d47b 100644 --- a/packages/plugin-detail/src/DetailSection.tsx +++ b/packages/plugin-detail/src/DetailSection.tsx @@ -27,6 +27,8 @@ import { } from '@object-ui/components'; import { ChevronDown, ChevronRight, Copy, Check, Eye, EyeOff, Pencil } from 'lucide-react'; import { SchemaRenderer, toRenderableSchema, useInlineEdit } from '@object-ui/react'; +import { resolveFieldAffordance } from '@object-ui/core'; +import { usePermissions } from '@object-ui/permissions'; import { getCellRenderer, resolveCellRendererType, @@ -167,6 +169,12 @@ export const DetailSection: React.FC = ({ * session to read, exactly as it has no draft. */ const serverFieldErrors = useInlineEdit()?.fieldErrors ?? null; + /** + * The caller's permissions, for the field question in-place edit asks of each + * field (objectui#12103). With no provider mounted `isLoaded` is false and + * the question is not asked, so a bare `DetailSection` behaves as before. + */ + const perms = usePermissions(); /** * What the copy affordance WRITES (objectui#8395). @@ -378,7 +386,16 @@ export const DetailSection: React.FC = ({ // clipboard are the READ direction of the same refusal, and `isInlineExcluded` // (objectui#4221) already holds the WRITE direction of it. const isMaskedField = isMaskedDetailFieldType(field.type, objectDefField?.type); - const fieldEditable = !isReadonly && !isComputedField && !isSystemField && !isInlineExcluded; + // The CALLER's write grant on this field (objectui#12103): the field + // question of the `recordEdit` row of the affordance-to-grant map, the same + // `write` question an edit form asks (`fieldWriteGate`). A field the + // permission set marks `editable: false` — or the owner field without the + // transfer grant — stays a read display, so in-place edit never offers a + // write the server refuses with 403. The flags above are the object's; this + // one is the principal's, and it only narrows. + const isWriteGranted = !objectName || resolveFieldAffordance('recordEdit', perms, objectName, field.name); + const fieldEditable = + !isReadonly && !isComputedField && !isSystemField && !isInlineExcluded && isWriteGranted; const canInlineEditField = fieldEditable && !!onEnterInlineEdit; const displayValue = (() => { diff --git a/packages/plugin-detail/src/HeaderHighlight.tsx b/packages/plugin-detail/src/HeaderHighlight.tsx index ef977ce211..1adf885de5 100644 --- a/packages/plugin-detail/src/HeaderHighlight.tsx +++ b/packages/plugin-detail/src/HeaderHighlight.tsx @@ -17,7 +17,8 @@ import { TooltipTrigger, } from '@object-ui/components'; import type { HighlightField } from '@object-ui/types'; -import { EXPANDABLE_FIELD_TYPES } from '@object-ui/core'; +import { EXPANDABLE_FIELD_TYPES, resolveFieldAffordance } from '@object-ui/core'; +import { usePermissions } from '@object-ui/permissions'; import { getCellRenderer, resolveCellRendererType, useBooleanValueLabel } from '@object-ui/fields'; import { useSafeFieldLabel, useInlineEdit } from '@object-ui/react'; import { Check, X, Pencil } from 'lucide-react'; @@ -63,6 +64,9 @@ export const HeaderHighlight: React.FC = ({ const inline = useInlineEdit(); const editing = inline?.editing ?? false; const canEdit = inline?.canEdit ?? false; + // The caller's permissions, for the field question the strip asks of each + // highlight — the same one the details body asks (objectui#12103). + const perms = usePermissions(); // The STAGED record: the saved record this strip receives, overlaid with the // edit session's draft by the same spread `DetailView` gives the details body @@ -155,7 +159,16 @@ export const HeaderHighlight: React.FC = ({ field.type, objectDefField?.type, ); - const fieldEditable = !isComputed && !isReadonly && !isSystem && !isInlineExcluded; + // The CALLER's write grant on this field (objectui#12103): the + // `recordEdit` row's field question, asked exactly as the details + // body asks it, since the strip and the body are one edit session + // with one Save. A field refused for this caller — `editable: false` + // in their permission set, or the owner without the transfer grant + // — exposes no editor here either. + const isWriteGranted = + !objectName || resolveFieldAffordance('recordEdit', perms, objectName, field.name); + const fieldEditable = + !isComputed && !isReadonly && !isSystem && !isInlineExcluded && isWriteGranted; const canInlineEditField = canEdit && fieldEditable; const editorActive = editing && canInlineEditField; diff --git a/packages/plugin-detail/src/__tests__/DetailSection.fieldWriteGrant-12103.test.tsx b/packages/plugin-detail/src/__tests__/DetailSection.fieldWriteGrant-12103.test.tsx new file mode 100644 index 0000000000..1d03940c97 --- /dev/null +++ b/packages/plugin-detail/src/__tests__/DetailSection.fieldWriteGrant-12103.test.tsx @@ -0,0 +1,185 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +import { describe, it, expect, vi, beforeAll, afterEach } from 'vitest'; +import { render, screen, fireEvent, cleanup } from '@testing-library/react'; +import React from 'react'; +import { InlineEditProvider, useInlineEdit } from '@object-ui/react'; +import { MePermissionsProvider, type MePermissionsResponse } from '@object-ui/permissions'; +import { SystemFieldName } from '@objectstack/spec/system'; +import type { DetailViewSection } from '@object-ui/types'; +import { DetailSection } from '../DetailSection'; +import { HeaderHighlight } from '../HeaderHighlight'; + +/** + * objectui#12103 — the record page's in-place edit asks the CALLER's field + * write grant. + * + * Measured on a live backend before this change: for a caller whose permission + * set reads `{ readable: true, editable: false }` for a field, and whose object + * grant carries `allowEdit: true` and `allowTransfer: false`, + * `/api/v1/auth/me/permissions` answers exactly that, and the server refuses a + * PATCH of the field ("Field write denied") and of the owner ("requires the + * transfer grant") with 403, while a PATCH of an ordinary field succeeds. The + * details body and the highlights strip still offered an editor for both. + * + * Both surfaces now ask the field question of the `recordEdit` row of the + * affordance-to-grant map — the same `write` question an edit form asks — so + * the envelope below is the measured one, not an invented shape. Assertions + * are behavioural (does double-click enter edit; does edit mode draw an input) + * and do not depend on the localized hint text. + */ + +const OBJECT = 'crm_account'; +const OWNER = SystemFieldName.OWNER_ID; + +/** The measured envelope: may edit the object, may not edit annual revenue, may not transfer. */ +function envelope(objectBits: Record = {}): MePermissionsResponse { + return { + authenticated: true, + userId: 'u-rep', + tenantId: null, + roles: [], + permissionSets: ['rep'], + objects: { + [OBJECT]: { + allowRead: true, + allowCreate: false, + allowEdit: true, + allowDelete: false, + allowTransfer: false, + ...objectBits, + }, + }, + fields: { [`${OBJECT}.annual_revenue`]: { readable: true, editable: false } }, + }; +} + +const objectSchema = { + fields: { + name: { type: 'text' }, + annual_revenue: { type: 'text' }, + // The owner as the platform injects it: an editable lookup to sys_user. + [OWNER]: { type: 'lookup', reference: 'sys_user' }, + }, +}; + +const data = { name: 'Acme', annual_revenue: '5000', [OWNER]: 'Alice' }; + +const section = { + fields: [ + { name: 'name', label: 'Name' }, + { name: 'annual_revenue', label: 'Annual revenue' }, + { name: OWNER, label: 'Owner' }, + ], +} as DetailViewSection; + +/** Render under the measured grant, or under no provider at all. */ +function withPerms(perms: MePermissionsResponse | null, node: React.ReactNode) { + return render(perms ? {node} : <>{node}); +} + +beforeAll(() => { + // useIsMobile() keys off window.innerWidth; pin a desktop width so the + // read-mode desktop row (which carries the affordance) renders. + Object.defineProperty(window, 'innerWidth', { configurable: true, value: 1280 }); +}); + +afterEach(cleanup); + +describe('the details body asks the field write grant (objectui#12103)', () => { + function enteredOnDoubleClick(perms: MePermissionsResponse | null, text: string): string[] { + const onEnter = vi.fn(); + withPerms( + perms, + , + ); + fireEvent.doubleClick(screen.getByText(text)); + return onEnter.mock.calls.map((c) => c[0]); + } + + it('a field marked editable: false offers no in-place edit', () => { + expect(enteredOnDoubleClick(envelope(), '5000')).toEqual([]); + }); + + it('the owner field without the transfer grant offers no in-place edit', () => { + expect(enteredOnDoubleClick(envelope(), 'Alice')).toEqual([]); + }); + + it('the owner field WITH the transfer grant does', () => { + expect(enteredOnDoubleClick(envelope({ allowTransfer: true }), 'Alice')).toEqual([OWNER]); + }); + + it('CONTROL: an editable field is unchanged', () => { + expect(enteredOnDoubleClick(envelope(), 'Acme')).toEqual(['name']); + }); + + it('CONTROL: with no permission provider every field edits, as before', () => { + expect(enteredOnDoubleClick(null, '5000')).toEqual(['annual_revenue']); + cleanup(); + expect(enteredOnDoubleClick(null, 'Alice')).toEqual([OWNER]); + }); + + it('in edit mode the refused field stays a read display while the editable one is an input', () => { + withPerms( + envelope(), + , + ); + expect(screen.getByDisplayValue('Acme')).toBeInTheDocument(); + expect(screen.queryByDisplayValue('5000')).toBeNull(); + expect(screen.getByText('5000')).toBeInTheDocument(); + }); +}); + +describe('the highlights strip asks the same question (objectui#12103)', () => { + /** Surfaces the shared session, so a test can read whether double-click entered it. */ + function SessionProbe() { + const inline = useInlineEdit(); + return
{inline?.editing ? `editing:${inline.autoFocusField ?? ''}` : 'idle'}
; + } + + function strip(perms: MePermissionsResponse | null) { + return withPerms( + perms, + + + + , + ); + } + + it('double-clicking a refused highlight does not enter the edit session', () => { + strip(envelope()); + fireEvent.doubleClick(screen.getByText('5000')); + expect(screen.getByTestId('session').textContent).toBe('idle'); + }); + + it('nor does the owner without the transfer grant; with it, it does', () => { + strip(envelope()); + fireEvent.doubleClick(screen.getByText('Alice')); + expect(screen.getByTestId('session').textContent).toBe('idle'); + cleanup(); + strip(envelope({ allowTransfer: true })); + fireEvent.doubleClick(screen.getByText('Alice')); + expect(screen.getByTestId('session').textContent).toBe(`editing:${OWNER}`); + }); + + it('CONTROL: an editable highlight enters the session and becomes an input', () => { + strip(envelope()); + fireEvent.doubleClick(screen.getByText('Acme')); + expect(screen.getByTestId('session').textContent).toBe('editing:name'); + expect(screen.getByDisplayValue('Acme')).toBeInTheDocument(); + }); + + it('once the session is open, the refused highlight stays a read display', () => { + strip(envelope()); + fireEvent.doubleClick(screen.getByText('Acme')); + expect(screen.queryByDisplayValue('5000')).toBeNull(); + expect(screen.getByText('5000')).toBeInTheDocument(); + }); +}); diff --git a/packages/plugin-form/src/affordanceGrantMap-12082.test.tsx b/packages/plugin-form/src/affordanceGrantMap-12082.test.tsx index 494cfc8476..8b993d7698 100644 --- a/packages/plugin-form/src/affordanceGrantMap-12082.test.tsx +++ b/packages/plugin-form/src/affordanceGrantMap-12082.test.tsx @@ -68,7 +68,8 @@ import { type FieldAffordance, } from '@object-ui/core'; import { MePermissionsProvider, usePermissions, type MePermissionsResponse } from '@object-ui/permissions'; -import { closedFormAffordance, gateFormFields } from './fieldWriteGate'; +import { SystemFieldName } from '@objectstack/spec/system'; +import { closedFormAffordance, fieldWriteGate, gateFormFields } from './fieldWriteGate'; import ts from 'typescript'; // The ONE answer to "is this span a comment, or code?" — a census that strips // comments with a private regex reports clean over code a phantom comment ate. @@ -118,6 +119,10 @@ const EXPECTED_FIELD_GRANT: Record = { createFormFields: 'create', editFormFields: 'update', listImport: 'create', + // objectui#12103: the record page's in-place edit and a list's in-cell edit + // offer fields to write, so each asks the update question of every field. + recordEdit: 'update', + listInlineEdit: 'update', }; const OBJECT = 'pin_object'; @@ -274,6 +279,119 @@ describe('fail-open: no permission provider mounted (objectui#12082)', () => { }); }); +// --------------------------------------------------------------------------- +// objectui#12103: the record page's in-place edit (`recordEdit`) and a list's +// in-cell edit (`listInlineEdit`) ask each field the SAME question an edit form +// asks, and the owner field's update needs the transfer grant on all of them. +// Measured first on a live backend: `/me/permissions` reported +// `editable: false` for the field and `allowTransfer: false` for the object, +// and the server refused both writes with 403, so the client follows an answer +// the server already gives. +// --------------------------------------------------------------------------- + +/** The owner field, as the spec names it and the server's transfer guard keys it. */ +const OWNER = SystemFieldName.OWNER_ID; + +/** The field rows that ask the update question, and those that ask the insert one. */ +const WRITE_FIELD_ROWS = FIELD_ROWS.filter((r) => AFFORDANCE_GRANTS[r].field === 'write'); +const CREATE_FIELD_ROWS = FIELD_ROWS.filter((r) => AFFORDANCE_GRANTS[r].field === 'create'); + +/** A caller who may edit the object but holds no transfer grant unless one is added. */ +const EDITOR = { allowCreate: true, allowRead: true, allowEdit: true, allowDelete: false }; + +describe("in-place edit and in-cell edit ask an edit form's field question (objectui#12103)", () => { + it('the rows that ask the update question are the edit form, the in-place edit and the in-cell edit', () => { + expect([...WRITE_FIELD_ROWS].sort()).toEqual(['editFormFields', 'listInlineEdit', 'recordEdit']); + }); + + it.each(SHAPES)("under a $shape grant each answers every field exactly as an edit form's gate does", ({ bits }) => { + const perms = principalUnder( + envelope({ ...bits, allowTransfer: false }, { + [`${OBJECT}.annual_revenue`]: { readable: true, editable: false }, + [`${OBJECT}.name`]: { readable: true, editable: true }, + }), + ); + // `fieldWriteGate` is what every ObjectForm layout hands its outbound + // strip: the predicate the card names as the one to follow. + const gate = fieldWriteGate(perms, OBJECT, 'edit'); + expect(gate).toBeDefined(); + for (const field of ['annual_revenue', 'name', 'any_field', OWNER]) { + for (const row of ['recordEdit', 'listInlineEdit'] as const) { + expect(resolveFieldAffordance(row, perms, OBJECT, field), `${row} ${field}`).toBe(gate!(field)); + } + } + }); + + it('a field marked editable: false is refused by both; CONTROL: an editable field is open on both', () => { + const perms = principalUnder( + envelope(EDITOR, { + [`${OBJECT}.annual_revenue`]: { readable: true, editable: false }, + [`${OBJECT}.name`]: { readable: true, editable: true }, + }), + ); + for (const row of ['recordEdit', 'listInlineEdit'] as const) { + expect(resolveFieldAffordance(row, perms, OBJECT, 'annual_revenue'), row).toBe(false); + expect(resolveFieldAffordance(row, perms, OBJECT, 'name'), row).toBe(true); + expect(resolveFieldAffordance(row, perms, OBJECT, 'any_field'), row).toBe(true); + } + }); +}); + +describe('the owner field: an update that writes it needs the transfer grant (objectui#12103)', () => { + it('without the transfer grant every update-question row offers the owner read-only — absent and false alike', () => { + const transfers: Array> = [{ allowTransfer: false }, {}]; + for (const transfer of transfers) { + const perms = principalUnder(envelope({ ...EDITOR, ...transfer })); + for (const row of WRITE_FIELD_ROWS) { + expect(resolveFieldAffordance(row, perms, OBJECT, OWNER), `${row} ${JSON.stringify(transfer)}`).toBe(false); + } + } + }); + + it('allowTransfer, or modifyAllRecords which implies it, opens the owner on every update-question row', () => { + const grants: Array> = [{ allowTransfer: true }, { modifyAllRecords: true }]; + for (const grant of grants) { + const perms = principalUnder(envelope({ ...EDITOR, ...grant })); + for (const row of WRITE_FIELD_ROWS) { + expect(resolveFieldAffordance(row, perms, OBJECT, OWNER), `${row} ${JSON.stringify(grant)}`).toBe(true); + } + } + }); + + it('CONTROL: the transfer grant moves no other field', () => { + for (const transfer of [false, true]) { + const perms = principalUnder(envelope({ ...EDITOR, allowTransfer: transfer })); + for (const row of WRITE_FIELD_ROWS) { + expect(resolveFieldAffordance(row, perms, OBJECT, 'name'), `${row} transfer=${transfer}`).toBe(true); + } + } + }); + + it('an explicit editable: false entry on the owner refuses it even with the transfer grant', () => { + const perms = principalUnder( + envelope({ ...EDITOR, allowTransfer: true }, { [`${OBJECT}.${OWNER}`]: { readable: true, editable: false } }), + ); + for (const row of WRITE_FIELD_ROWS) { + expect(resolveFieldAffordance(row, perms, OBJECT, OWNER), row).toBe(false); + } + }); + + it('the insert question is not narrowed: on insert the server stamps the caller as owner and accepts their own id', () => { + const perms = principalUnder(envelope({ ...EDITOR, allowTransfer: false })); + expect(CREATE_FIELD_ROWS.length).toBeGreaterThan(0); + for (const row of CREATE_FIELD_ROWS) { + expect(resolveFieldAffordance(row, perms, OBJECT, OWNER), row).toBe(true); + } + }); + + it('with no permission provider the owner reads open, as every field question does', () => { + const perms = principalUnder(null); + for (const row of WRITE_FIELD_ROWS) { + expect(resolveFieldAffordance(row, perms, OBJECT, OWNER), row).toBe(true); + } + }); +}); + // --------------------------------------------------------------------------- // The census: no console source reads a CRUD grant outside the map. // @@ -480,6 +598,8 @@ const DETAIL_VIEW = 'packages/plugin-detail/src/DetailView.tsx'; const OBJECT_GRID = 'packages/plugin-grid/src/ObjectGrid.tsx'; const RECORD_PAGE = 'packages/app-shell/src/views/RecordDetailView.tsx'; const RECORD_DETAILS = 'packages/plugin-detail/src/renderers/record-details.tsx'; +const DETAIL_SECTION = 'packages/plugin-detail/src/DetailSection.tsx'; +const HEADER_HIGHLIGHT = 'packages/plugin-detail/src/HeaderHighlight.tsx'; const CONSOLE_LIST = 'packages/app-shell/src/views/ObjectView.tsx'; const LIST_VIEW = 'packages/plugin-list/src/ListView.tsx'; @@ -583,8 +703,10 @@ const WRITE_SITES: Record = { 'packages/plugin-designer/src/pages/DashboardDesignPage.tsx :: DashboardDesignPage > saveSchema :: update': unmapped("the dashboard designer's Save updates a sys_dashboard row and reads no grant on sys_dashboard"), // ── plugin-detail ── - 'packages/plugin-detail/src/InlineEditSaveBar.tsx :: updateVia :: update': - mapped(['recordEdit'], RECORD_PAGE, RECORD_DETAILS), + 'packages/plugin-detail/src/InlineEditSaveBar.tsx :: updateVia :: update': { + ...mapped(['recordEdit'], RECORD_PAGE, RECORD_DETAILS, DETAIL_SECTION, HEADER_HIGHLIGHT), + note: 'the page and the record body read the object verdict; the details body and the highlights strip ask each field the row\'s field question (objectui#12103)', + } as WriteSite, 'packages/plugin-detail/src/RelatedList.tsx :: RelatedList > handleAddRecords :: create': unmapped("a related list's Add existing (picker) creates a junction row and reads no grant on it"), 'packages/plugin-detail/src/RelatedList.tsx :: RelatedList > handleAddRecords :: update': diff --git a/packages/plugin-grid/src/ObjectGrid.tsx b/packages/plugin-grid/src/ObjectGrid.tsx index 580aa75584..36bfe509aa 100644 --- a/packages/plugin-grid/src/ObjectGrid.tsx +++ b/packages/plugin-grid/src/ObjectGrid.tsx @@ -58,7 +58,7 @@ import { DataEmptyState, resolveIcon, } from '@object-ui/components'; import { usePullToRefresh } from '@object-ui/mobile'; -import { resolveConditionalFormatting, leadWithNameField, buildExpandFields, buildExportFileName, columnIdentity, collectPredicateFieldRefs, collectGroupingFieldRefs, listViewPredicates, resolveAffordance, type SchemaLike, isProjectableField, isExpandableFieldType, isUnmaterializedFieldType, readObjectSortability, isPlatformSortableField, filterPlatformSortableSort, toFilterNode, toFilterNodeSafely, filterRefusalSubject, FilterOperatorError, convertSortToQueryParams, normalizeSortEntries, type QuerySortEntry, ROW_HEIGHT_TO_DENSITY_MODE, resolveRecordSourceConfig, resolveRecordSourceObjectName, resolveFilterPlaceholders, partitionRowsByPredicate, buildCategoryOrder, buildCategoryRank, type FilterTokenScope } from '@object-ui/core'; +import { resolveConditionalFormatting, leadWithNameField, buildExpandFields, buildExportFileName, columnIdentity, collectPredicateFieldRefs, collectGroupingFieldRefs, listViewPredicates, resolveAffordance, resolveFieldAffordance, type SchemaLike, isProjectableField, isExpandableFieldType, isUnmaterializedFieldType, readObjectSortability, isPlatformSortableField, filterPlatformSortableSort, toFilterNode, toFilterNodeSafely, filterRefusalSubject, FilterOperatorError, convertSortToQueryParams, normalizeSortEntries, type QuerySortEntry, ROW_HEIGHT_TO_DENSITY_MODE, resolveRecordSourceConfig, resolveRecordSourceObjectName, resolveFilterPlaceholders, partitionRowsByPredicate, buildCategoryOrder, buildCategoryRank, type FilterTokenScope } from '@object-ui/core'; import { usePermissions } from '@object-ui/permissions'; import { RECORD_OVERLAY_DEFAULT_WIDTH, @@ -4455,7 +4455,21 @@ export const ObjectGrid: React.FC = ({ // mark the column so the data-table never opens an editor (otherwise it // falls back to a plain text box for e.g. a Formula or File cell). Only // force `false`; leave editable unset otherwise so the grid-level flag wins. - if (next.editable !== false && !isFieldInlineEditable(fieldDef)) { + // + // [objectui#12103] …and neither is a field THIS CALLER may not write: the + // field question of the `listInlineEdit` row of the affordance-to-grant map, + // the same `write` question an edit form asks (`fieldWriteGate`). A field + // the permission set marks `editable: false`, or the owner field without + // the transfer grant, keeps a read cell while the rest of the row edits; + // before, its editor opened and the save answered 403. `inlineEditable` + // above stays the object-level verdict for the whole grid — this only + // narrows one column. With no `objectName`, or no provider mounted, nothing + // is asked and the column is untouched. + if ( + next.editable !== false && + (!isFieldInlineEditable(fieldDef) || + (!!objectName && !resolveFieldAffordance('listInlineEdit', perms, objectName, col.accessorKey))) + ) { next.editable = false; } return next; diff --git a/packages/plugin-grid/src/__tests__/inlineEditFieldWriteGrant-12103.test.tsx b/packages/plugin-grid/src/__tests__/inlineEditFieldWriteGrant-12103.test.tsx new file mode 100644 index 0000000000..31304e389f --- /dev/null +++ b/packages/plugin-grid/src/__tests__/inlineEditFieldWriteGrant-12103.test.tsx @@ -0,0 +1,180 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * objectui#12103 — a list's in-cell edit asks the CALLER's field write grant. + * + * `inlineEditPermissionGate.test.tsx` owns the OBJECT-level half (the grid is + * editable at all only when the caller holds `update`), and stubs the field + * question out of the way. This file owns the FIELD-level half, through the + * REAL `MePermissionsProvider` and the envelope measured on a live backend: + * `allowEdit: true`, `allowTransfer: false`, and + * `{ readable: true, editable: false }` for one field. The server refuses a + * PATCH of that field and of the owner with 403; before this change both cells + * opened an editor anyway. + * + * The observable is the user's: does clicking the cell open an editor, and + * does the cell carry the editable cursor `data-table` draws only for a column + * it will open. + */ + +import { describe, it, expect, vi, beforeAll, beforeEach, afterEach } from 'vitest'; +import { render, screen, waitFor, fireEvent, cleanup } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import React from 'react'; +import { MePermissionsProvider, type MePermissionsResponse } from '@object-ui/permissions'; +import { SystemFieldName } from '@objectstack/spec/system'; +import { ObjectGrid } from '../ObjectGrid'; +import { __clearRecordCrudVerdictCache } from '../hooks/useRecordCrudVerdicts'; +import { installExplainDouble } from './explainDouble'; +import { registerAllFields } from '@object-ui/fields'; +import { ActionProvider, SchemaRendererProvider } from '@object-ui/react'; + +registerAllFields(); + +const OBJECT = 'crm_account'; +const OWNER = SystemFieldName.OWNER_ID; + +beforeAll(() => { + if (!Element.prototype.scrollIntoView) { + Element.prototype.scrollIntoView = vi.fn() as any; + } +}); + +beforeEach(() => { + __clearRecordCrudVerdictCache(); + installExplainDouble(); +}); +afterEach(() => { + cleanup(); + vi.unstubAllGlobals(); +}); + +/** The measured envelope: may edit the object, may not edit annual revenue, may not transfer. */ +function envelope(objectBits: Record = {}): MePermissionsResponse { + return { + authenticated: true, + userId: 'u-rep', + tenantId: null, + roles: [], + permissionSets: ['rep'], + objects: { + [OBJECT]: { + allowRead: true, + allowCreate: false, + allowEdit: true, + allowDelete: false, + allowTransfer: false, + ...objectBits, + }, + }, + fields: { [`${OBJECT}.annual_revenue`]: { readable: true, editable: false } }, + }; +} + +function makeDataSource() { + const store: Record = { + r1: { id: 'r1', name: 'Acme', annual_revenue: '5000', [OWNER]: 'Alice' }, + }; + return { + find: vi.fn(async () => { + const data = Object.values(store).map((r) => ({ ...r })); + return { data, total: data.length, hasMore: false, pageSize: 50 }; + }), + findOne: vi.fn(async (_o: string, id: string) => ({ ...store[id] })), + update: vi.fn(async (_o: string, id: string, changes: Record) => ({ ...store[id], ...changes })), + getObjectSchema: async (name: string) => ({ + name, + fields: { + id: { type: 'text' }, + name: { type: 'text', label: 'Name' }, + annual_revenue: { type: 'text', label: 'Annual revenue' }, + // Typed `text` here so the cell's editor is an the test can + // see; the platform's owner is a lookup, and the gate keys on the + // field, not on its type. + [OWNER]: { type: 'text', label: 'Owner' }, + }, + }), + } as any; +} + +function renderGrid(perms: MePermissionsResponse | null) { + const ds = makeDataSource(); + const schema: any = { + type: 'object-grid', + objectName: OBJECT, + editable: true, + singleClickEdit: true, + columns: [ + { field: 'name', label: 'Name', type: 'text' }, + { field: 'annual_revenue', label: 'Annual revenue', type: 'text' }, + { field: OWNER, label: 'Owner', type: 'text' }, + ], + pagination: { pageSize: 50 }, + }; + const grid = ( + + + {}} onDelete={() => {}} /> + + + ); + return render(perms ? {grid} : grid); +} + +/** The cell showing `text` in the first body row. */ +async function cell(text: string): Promise { + await waitFor(() => expect(screen.getByText(text)).toBeInTheDocument()); + const row = document.querySelector('tbody tr') as HTMLElement; + const td = Array.from(row.querySelectorAll('td')).find((c) => c.textContent?.trim() === text); + if (!td) throw new Error(`no cell reads ${text}`); + return td as HTMLElement; +} + +/** Click the cell and report whether an in-cell editor opened. */ +async function opensEditor(text: string): Promise { + const td = await cell(text); + // `data-table` draws the editable cursor only on a column it will open. + const offered = td.className.includes('cursor-text'); + fireEvent.click(td); + await new Promise((r) => setTimeout(r, 50)); + const opened = td.querySelector('input') != null; + // The two observables agree, or the probe cannot tell the shapes apart. + expect(opened, `${text}: cursor and editor disagree`).toBe(offered); + return opened; +} + +describe("a list's in-cell edit asks the field write grant (objectui#12103)", () => { + it('a field marked editable: false keeps a read cell', async () => { + renderGrid(envelope()); + expect(await opensEditor('5000')).toBe(false); + }); + + it('the owner field without the transfer grant keeps a read cell', async () => { + renderGrid(envelope()); + expect(await opensEditor('Alice')).toBe(false); + }); + + it('the owner field WITH the transfer grant opens its editor', async () => { + renderGrid(envelope({ allowTransfer: true })); + expect(await opensEditor('Alice')).toBe(true); + }); + + it('CONTROL: an editable field in the same row is unchanged', async () => { + renderGrid(envelope()); + expect(await opensEditor('Acme')).toBe(true); + }); + + it('CONTROL: with no permission provider every column edits, as before', async () => { + renderGrid(null); + expect(await opensEditor('5000')).toBe(true); + cleanup(); + renderGrid(null); + expect(await opensEditor('Alice')).toBe(true); + }); +}); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 2c8b158ae6..fb1a4949ab 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1457,6 +1457,9 @@ importers: '@object-ui/types': specifier: workspace:* version: link:../types + '@objectstack/spec': + specifier: ^17.7.0 + version: 17.7.0(ai@7.0.65(zod@4.6.5)) devDependencies: '@types/react': specifier: 19.2.18