diff --git a/.changeset/12109-nav-object-read-gate.md b/.changeset/12109-nav-object-read-gate.md new file mode 100644 index 0000000000..7eb3f999b1 --- /dev/null +++ b/.changeset/12109-nav-object-read-gate.md @@ -0,0 +1,12 @@ +--- +'@object-ui/app-shell': patch +--- + +**The console no longer offers an object to a caller who cannot read it** (objectui#12109). A navigation entry bound to an object (`type: 'object'`, by its `objectName`) is not drawn in the sidebar or the `nav:menu` block when the caller has no read on that object, and the object's list toolbar actions are not drawn on its list page. Before, a service agent with no read on opportunities saw Opportunities, My Deals and Update Stage, and each one ended on "You don't have access". + +- **The verdict** is the permissions the console already loads (`/auth/me/permissions`). No request is made per entry. It is the answer an authored `requiredPermissions: ['OBJECT:read']` already got, now applied by default, and an authored `requiredPermissions` still applies on top. +- **An unknown answer hides nothing.** With no permission provider mounted, every entry is drawn as before. +- **Only `type: 'object'` entries are gated.** A dashboard, report, page, component, action, doc or URL entry is unchanged: none of them names an object in a declared key, and no object is guessed from a route. +- **An area** whose only entries are such objects is not offered in the area switcher, and is not elected as the open area. + +**Clause-②: no.** No export, prop, locale key or accept set moves. diff --git a/packages/app-shell/src/layout/UnifiedSidebar.tsx b/packages/app-shell/src/layout/UnifiedSidebar.tsx index bdcf01fdbf..dbd28391a1 100644 --- a/packages/app-shell/src/layout/UnifiedSidebar.tsx +++ b/packages/app-shell/src/layout/UnifiedSidebar.tsx @@ -52,6 +52,7 @@ import { useNavActionDispatch } from '../hooks/useNavActionDispatch.js'; import { useNavTargetLabel } from '../hooks/useNavTargetLabel.js'; import { useNavDocTargetCheck } from '../hooks/useNavDocTargetCheck.js'; import { matchAppBySegment, appRouteSegment, resolveKeyedI18nLabel } from '../utils/index.js'; +import { withoutUnreadableObjectEntries } from '../utils/navObjectReadGate.js'; import { useHomePath } from '../hooks/useHomePath.js'; // Aliased for symmetry with objectui's own `resolveKeyedI18nLabel` above (the // names stopped colliding in objectui#4167): this is the spec's resolver (new in @@ -231,6 +232,23 @@ export function UnifiedSidebar({ activeAppName }: UnifiedSidebarProps) { // an area that renders nothing. Same derivation as `AppSchemaRenderer` // (@object-ui/layout); the predicate is shared, not re-implemented. const areas: NavigationArea[] = activeApp?.areas || []; + // An entry bound to an object the caller may not read is not drawn + // (objectui#12109): the tree is pruned HERE, before it reaches the layout, + // because no callback the layout asks answers "may this member read this + // object" (see `withoutUnreadableObjectEntries`). Every area's tree and the + // flat tree are pruned the same way, so the area election below and the drawn + // menu read the same tree. Not memoised: a tree with nothing to prune comes + // back as the very array it was, so a caller who may read everything hands + // the layout the authored tree, and nothing downstream keys on the identity + // of a pruned one (AGENTS.md #10). + const readableAreas = areas.map((area) => { + if (!area.navigation) return area; + const navigation = withoutUnreadableObjectEntries(area.navigation, can); + return navigation === area.navigation ? area : { ...area, navigation }; + }); + const readableAppNavigation: NavigationItem[] | undefined = activeApp?.navigation + ? withoutUnreadableObjectEntries(activeApp.navigation, can) + : activeApp?.navigation; // A `doc` entry the member may not read is not drawn (objectui#10188) — // defence in depth behind the server's app read, which already drops it // (objectstack#19790). Asked only in an app: on Home `activeApp` is merely @@ -238,7 +256,7 @@ export function UnifiedSidebar({ activeAppName }: UnifiedSidebarProps) { const checkDocTarget = useNavDocTargetCheck( context === 'app' ? [activeApp?.navigation, ...areas.map((area) => area.navigation)] : [], ); - const visibleAreas = areas.filter((area) => + const visibleAreas = readableAreas.filter((area) => hasVisibleNavigationItems(area.navigation, { evaluateVisibility: evalVis, checkPermission: checkPerm, @@ -271,7 +289,7 @@ export function UnifiedSidebar({ activeAppName }: UnifiedSidebarProps) { // covers the frame between a gating change hiding the active area and the // effect above re-electing. const activeArea = visibleAreas.find((a) => a.id === activeAreaId) ?? visibleAreas[0]; - const appNavigation: NavigationItem[] = activeArea?.navigation || activeApp?.navigation || []; + const appNavigation: NavigationItem[] = activeArea?.navigation || readableAppNavigation || []; // App-level context selectors (e.g. Studio's package scope). Their // values are injected into nav items as `{}` template vars so a diff --git a/packages/app-shell/src/layout/__tests__/UnifiedSidebar.objectReadGate-12109.test.tsx b/packages/app-shell/src/layout/__tests__/UnifiedSidebar.objectReadGate-12109.test.tsx new file mode 100644 index 0000000000..4de6e83de0 --- /dev/null +++ b/packages/app-shell/src/layout/__tests__/UnifiedSidebar.objectReadGate-12109.test.tsx @@ -0,0 +1,245 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * UnifiedSidebar — an entry bound to an object the caller may not read is not + * drawn, by default (objectui#12109). + * + * Measured on HotCRM (objectstack-ai/hotcrm#2058, 17.7.0): a service agent with + * no read on `crm_opportunity` was shown Opportunities and My Deals, each + * ending in "You don't have access". The fixture below is that app's shape: + * both entries are `type: 'object'` on `crm_opportunity`, one in a group with a + * readable sibling, one alone in its group. + * + * The verdict is the REAL `MePermissionsProvider`'s, fed the `/me/permissions` + * answer through its `fetcher` — the permissions the console already loads — + * so this also measures that the gate costs no request per entry. + * + * Harness: the provider / chrome mocks of `UnifiedSidebar.docAudience-10188.test.tsx`, + * except that `@object-ui/permissions` is NOT mocked; `MetadataProvider`, + * `@object-ui/layout` and `@object-ui/components` stay real. + */ + +import '@testing-library/jest-dom/vitest'; +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import React from 'react'; +import { render, screen, waitFor } from '@testing-library/react'; +import { MemoryRouter } from 'react-router-dom'; +import type { NavigationArea, NavigationItem } from '@object-ui/types'; +import { MePermissionsProvider, type MePermissionsResponse } from '@object-ui/permissions'; + +vi.mock('@object-ui/i18n', async (importOriginal) => ({ + ...(await importOriginal>()), + useObjectTranslation: () => ({ + t: (key: string, options?: Record) => String(options?.defaultValue ?? key), + language: 'en', + }), + useObjectLabel: () => ({ + objectLabel: ({ label }: { label?: string }) => label, + viewLabel: (_o: string, _v: string, fallback?: string) => fallback, + dashboardLabel: ({ label }: { label?: string }) => label, + appLabel: ({ label }: { label?: string }) => label, + }), +})); + +vi.mock('@object-ui/auth', async (importOriginal) => ({ + ...(await importOriginal>()), + useAuth: () => ({ user: null, activeOrganization: null }), + useWorkspaceAdminStatus: () => ({ isAdmin: false, isResolved: true }), +})); + +vi.mock('../../providers/ExpressionProvider', () => ({ + useExpressionContext: () => ({ evaluator: null }), + evaluateVisibility: (expr: unknown) => expr !== false && expr !== 'false', +})); + +vi.mock('../../utils', () => ({ + resolveKeyedI18nLabel: (label: unknown) => (typeof label === 'string' ? label : ''), + matchAppBySegment: (apps: Array<{ name?: string }>, segment?: string) => + apps.find((a) => a?.name === segment), + appRouteSegment: (app: { name?: string }) => app?.name, +})); + +vi.mock('../../utils/getIcon', () => ({ getIcon: () => () => null })); +vi.mock('../../hooks/useRecentItems', () => ({ useRecentItems: () => ({ recentItems: [] }) })); +vi.mock('../../hooks/useFavorites', () => ({ + useFavorites: () => ({ favorites: [], removeFavorite: vi.fn() }), +})); +vi.mock('../../hooks/useNavPins', () => ({ + useNavPins: () => ({ togglePin: vi.fn(), applyPins: (items: unknown) => items }), +})); +vi.mock('../../hooks/useNavActionDispatch', () => ({ + useNavActionDispatch: () => vi.fn(), +})); +vi.mock('../../context/NavigationContext', () => ({ + useNavigationContext: () => ({ context: 'app', currentAppName: 'crm' }), +})); +vi.mock('../ContextSelectors', () => ({ + useAppContextSelectors: () => ({ contextValues: {}, element: null }), + contextSelectorQueryKey: (id: string) => (id === 'active_package' ? 'package' : id), + STUDIO_PACKAGE_SELECTOR_ID: 'active_package', +})); +vi.mock('../LocalizedSidebarTrigger', () => ({ + LocalizedSidebarTrigger: () => null, +})); + +import { SidebarProvider } from '@object-ui/components'; +import { MetadataProvider } from '../../providers/MetadataProvider'; +import { UnifiedSidebar } from '../UnifiedSidebar'; + +/** HotCRM's shape, abridged to the entries this card names and their neighbours. */ +const NAVIGATION: NavigationItem[] = [ + { id: 'nav_home', type: 'dashboard', dashboardName: 'crm_home', label: 'Home' }, + { + id: 'group_sales', + type: 'group', + label: 'Sales', + children: [ + { id: 'nav_account', type: 'object', objectName: 'crm_account', label: 'Accounts' }, + { id: 'nav_opportunity', type: 'object', objectName: 'crm_opportunity', label: 'Opportunities' }, + ], + }, + { + id: 'group_my_work', + type: 'group', + label: 'My Work', + children: [ + { id: 'nav_my_deals', type: 'object', objectName: 'crm_opportunity', viewName: 'my_open_deals', label: 'My Deals' }, + ], + }, + // Readable, but the author gated it on a grant this caller lacks. + { id: 'nav_contact', type: 'object', objectName: 'crm_contact', label: 'Contacts', requiredPermissions: ['crm_contact:delete'] }, + { id: 'nav_help', type: 'url', url: '/help', label: 'Help' }, +]; + +function answerFor(opportunityRead: boolean): MePermissionsResponse { + return { + authenticated: true, + userId: 'u_agent', + tenantId: null, + roles: [], + permissionSets: ['service_agent'], + systemPermissions: [], + objects: { + crm_account: { allowRead: true }, + crm_opportunity: { allowRead: opportunityRead }, + crm_contact: { allowRead: true, allowDelete: false }, + }, + fields: {}, + }; +} + +function adapterFor(app: Record) { + return { + clearCache: vi.fn(), + getClient: () => ({ + meta: { + getItems: (type: string) => Promise.resolve({ type, items: type === 'app' ? [app] : [] }), + getItem: () => Promise.resolve({ item: null }), + }, + }), + } as unknown as Parameters[0]['adapter']; +} + +function sidebar(app: Record) { + return ( + + + + + + + + ); +} + +const CRM_APP = { name: 'crm', label: 'CRM', active: true, navigation: NAVIGATION }; + +/** The console's mount: the sidebar inside the provider that loads `/me/permissions`. */ +function renderFor(answer: MePermissionsResponse, app: Record = CRM_APP) { + const fetcher = vi.fn(async () => ({ ok: true, status: 200, json: async () => answer }) as unknown as Response); + render( + + {sidebar(app)} + , + ); + return fetcher; +} + +const link = (name: string) => screen.queryByRole('link', { name }); + +beforeEach(() => { + localStorage.clear(); + sessionStorage.clear(); +}); + +describe('UnifiedSidebar — an object entry follows the caller\'s read on its object (objectui#12109)', () => { + it('the service agent: Opportunities and My Deals are not drawn; the readable entry is', async () => { + renderFor(answerFor(false)); + await waitFor(() => expect(link('Accounts')).toBeInTheDocument()); + expect(link('Opportunities')).not.toBeInTheDocument(); + expect(link('My Deals')).not.toBeInTheDocument(); + // A group whose only entry is gated away draws no heading either. + expect(screen.queryByText('My Work')).not.toBeInTheDocument(); + expect(screen.getByText('Sales')).toBeInTheDocument(); + }); + + it('a caller who may read the object: both entries are drawn, on the same app document', async () => { + renderFor(answerFor(true)); + await waitFor(() => expect(link('Opportunities')).toHaveAttribute('href', '/apps/crm/crm_opportunity')); + expect(link('My Deals')).toHaveAttribute('href', '/apps/crm/crm_opportunity/view/my_open_deals'); + expect(link('Accounts')).toBeInTheDocument(); + }); + + it('CONTROL: an entry that is not bound to an object is unchanged', async () => { + renderFor(answerFor(false)); + await waitFor(() => expect(link('Accounts')).toBeInTheDocument()); + expect(link('Home')).toHaveAttribute('href', '/apps/crm/dashboard/crm_home'); + expect(link('Help')).toBeInTheDocument(); + }); + + it('an authored `requiredPermissions` still applies on top of a readable object', async () => { + renderFor(answerFor(true)); + await waitFor(() => expect(link('Accounts')).toBeInTheDocument()); + expect(link('Contacts')).not.toBeInTheDocument(); + }); + + it('the verdict comes from the one permissions read the shell already makes, not a request per entry', async () => { + const fetcher = renderFor(answerFor(false)); + await waitFor(() => expect(link('Accounts')).toBeInTheDocument()); + await new Promise((resolve) => setTimeout(resolve, 50)); + expect(fetcher).toHaveBeenCalledTimes(1); + }); + + it('an area whose only entries are unreadable is not offered, and is not elected', async () => { + const areas: NavigationArea[] = [ + { + id: 'area_sales', + label: 'Pipeline', + navigation: [{ id: 'nav_opportunity', type: 'object', objectName: 'crm_opportunity', label: 'Opportunities' }], + }, + { + id: 'area_service', + label: 'Service', + navigation: [{ id: 'nav_account', type: 'object', objectName: 'crm_account', label: 'Accounts' }], + }, + { + id: 'area_insight', + label: 'Insight', + navigation: [{ id: 'nav_home', type: 'dashboard', dashboardName: 'crm_home', label: 'Home' }], + }, + ]; + renderFor(answerFor(false), { name: 'crm', label: 'CRM', active: true, navigation: [], areas }); + await waitFor(() => expect(link('Accounts')).toBeInTheDocument()); + expect(screen.getByText('Service')).toBeInTheDocument(); + expect(screen.getByText('Insight')).toBeInTheDocument(); + expect(screen.queryByText('Pipeline')).not.toBeInTheDocument(); + expect(link('Opportunities')).not.toBeInTheDocument(); + }); + + it('with no permission provider the answer is unknown, and an unknown hides nothing', async () => { + render(sidebar(CRM_APP)); + await waitFor(() => expect(link('Opportunities')).toBeInTheDocument()); + expect(link('My Deals')).toBeInTheDocument(); + expect(link('Accounts')).toBeInTheDocument(); + }); +}); diff --git a/packages/app-shell/src/utils/__tests__/navObjectReadGate-12109.test.ts b/packages/app-shell/src/utils/__tests__/navObjectReadGate-12109.test.ts new file mode 100644 index 0000000000..88f64be2ba --- /dev/null +++ b/packages/app-shell/src/utils/__tests__/navObjectReadGate-12109.test.ts @@ -0,0 +1,106 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectui#12109 — the console's default read gate on a navigation tree: + * `withoutUnreadableObjectEntries` drops a `type: 'object'` entry whose object + * the caller may not read, at any depth, and leaves every other entry as it is. + * + * The rendered surfaces that prune through it are pinned beside them + * (`UnifiedSidebar.objectReadGate-12109.test.tsx`, + * `nav-object-read-gate-12109.render.test.tsx`); this file pins the function's + * own contract, the `type: 'object'`-only reach in particular. + */ + +import { describe, it, expect, vi } from 'vitest'; +import type { NavigationItem } from '@object-ui/types'; +import { mayReadObject, withoutUnreadableObjectEntries, type ObjectReadCheck } from '../navObjectReadGate'; + +/** A service agent's answer: no read on opportunities, read on everything else. */ +const agent: ObjectReadCheck = (object) => object !== 'crm_opportunity'; + +const ids = (items: NavigationItem[]): string[] => + items.flatMap((item) => [item.id ?? '', ...(item.type === 'separator' ? [] : ids(item.children ?? []))]); + +describe('withoutUnreadableObjectEntries (objectui#12109)', () => { + it('drops an object entry the caller may not read, and keeps one they may read', () => { + const items: NavigationItem[] = [ + { id: 'nav_opportunity', type: 'object', objectName: 'crm_opportunity', label: 'Opportunities' }, + { id: 'nav_account', type: 'object', objectName: 'crm_account', label: 'Accounts' }, + ]; + expect(ids(withoutUnreadableObjectEntries(items, agent))).toEqual(['nav_account']); + }); + + it('drops it at every depth: inside a group, and an object entry takes its children with it', () => { + const items: NavigationItem[] = [ + { + id: 'group_sales', + type: 'group', + label: 'Sales', + children: [ + { id: 'nav_account', type: 'object', objectName: 'crm_account', label: 'Accounts' }, + { + id: 'nav_opportunity', + type: 'object', + objectName: 'crm_opportunity', + label: 'Opportunities', + children: [{ id: 'nav_pipeline', type: 'dashboard', dashboardName: 'pipeline', label: 'Pipeline' }], + }, + ], + }, + { + id: 'group_my_work', + type: 'group', + label: 'My Work', + children: [ + { id: 'nav_my_deals', type: 'object', objectName: 'crm_opportunity', viewName: 'my_open_deals', label: 'My Deals' }, + ], + }, + ]; + // The emptied group is KEPT: whether a group with no surviving child draws + // is the layout's item guard's question, and it answers "no". + expect(ids(withoutUnreadableObjectEntries(items, agent))).toEqual(['group_sales', 'nav_account', 'group_my_work']); + }); + + it('CONTROL: an entry that is not `type: object` is unchanged, whatever object its target reads', () => { + const items: NavigationItem[] = [ + { id: 'nav_dash', type: 'dashboard', dashboardName: 'crm_opportunity', label: 'Pipeline' }, + { id: 'nav_report', type: 'report', reportName: 'crm_opportunity', label: 'Report' }, + { id: 'nav_page', type: 'page', pageName: 'crm_opportunity', label: 'Page' }, + { id: 'nav_url', type: 'url', url: '/apps/crm/crm_opportunity', label: 'By URL' }, + { id: 'nav_action', type: 'action', label: 'Update Stage', actionDef: { actionName: 'mass_update_stage' } }, + { id: 'nav_component', type: 'component', componentRef: 'approvals:inbox', requiresObject: 'crm_opportunity', label: 'Inbox' }, + { id: 'nav_doc', type: 'doc', doc: 'crm_opportunity', label: 'Doc' }, + { id: 'sep', type: 'separator' }, + ]; + const kept = withoutUnreadableObjectEntries(items, agent); + expect(kept).toBe(items); + expect(ids(kept)).toEqual(ids(items)); + }); + + it('a caller who may read everything gets the very tree back, nested parents included', () => { + const group: NavigationItem = { + id: 'group_sales', + type: 'group', + label: 'Sales', + children: [{ id: 'nav_account', type: 'object', objectName: 'crm_account', label: 'Accounts' }], + }; + const items: NavigationItem[] = [group]; + const kept = withoutUnreadableObjectEntries(items, () => true); + expect(kept).toBe(items); + expect(kept[0]).toBe(group); + }); + + it('asks the verdict for the `read` action of the entry\'s own `objectName`, once per object entry', () => { + const can = vi.fn(() => true); + withoutUnreadableObjectEntries( + [ + { id: 'nav_account', type: 'object', objectName: 'crm_account', label: 'Accounts' }, + { id: 'nav_dash', type: 'dashboard', dashboardName: 'crm_lead', label: 'Leads board' }, + ], + can, + ); + expect(can.mock.calls).toEqual([['crm_account', 'read']]); + expect(mayReadObject(can, 'crm_lead')).toBe(true); + expect(can).toHaveBeenLastCalledWith('crm_lead', 'read'); + }); +}); diff --git a/packages/app-shell/src/utils/navObjectReadGate.ts b/packages/app-shell/src/utils/navObjectReadGate.ts new file mode 100644 index 0000000000..9683947846 --- /dev/null +++ b/packages/app-shell/src/utils/navObjectReadGate.ts @@ -0,0 +1,101 @@ +/** + * 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. + */ + +/** + * The console's default read gate on what an object opens (objectui#12109): + * a navigation entry bound to an object, and a list action bound to it, are not + * drawn for a caller who may not read that object. Before this, both were drawn + * and each ended on the list's "You don't have access". + * + * ## The verdict, and where it comes from + * + * {@link mayReadObject}: `can(objectName, 'read')` from `usePermissions()`, the + * permissions the console already holds (`MePermissionsProvider`, one + * `/auth/me/permissions` read for the whole shell). Nothing here makes a + * request. It is the answer the sidebar's permission checker already gives an + * AUTHORED `requiredPermissions: ['OBJECT:read']`, applied by default; an + * authored `requiredPermissions` still applies on top, in `@object-ui/layout`'s + * item guard. + * + * An unknown answer is not a refusal. With no permission provider mounted, + * `can` answers `true`. `MePermissionsProvider` renders its `loadingFallback`, + * not its children, until it holds an answer, and while it refetches it answers + * from the map it holds, exactly as it does for an authored + * `requiredPermissions`. So an entry neither flashes in nor flashes out. + * + * ## Which navigation entries are "bound to an object" + * + * `type: 'object'` only: the one navigation item whose spec shape names an + * object in a declared key (`objectName`). A `dashboard`, `report`, `page`, + * `component` or `action` entry names its own kind of target; which objects + * that target reads is the target's own question, and no object is guessed + * from a route string. An object entry's `children` go with it, as a gated + * node's subtree does in the layout's item guard. + * + * ## Why the host prunes, and the layout does not ask + * + * The server leaves object visibility to the client by design (objectstack's + * `filterAppForUserWithReason` docblock). `@object-ui/layout` holds no + * permissions: it asks its host through callbacks, and none of them asks + * whether the MEMBER may read an object (`CapabilityChecker` asks whether the + * RUNTIME has one). So the host hands the layout an already-pruned tree, and + * every path through the layout reads the same tree: the drawn rows, the + * derived area election and the Favorites collection. `UnifiedSidebar` and the + * `nav:menu` block prune through {@link withoutUnreadableObjectEntries}, so the + * two menus agree. + * + * Internal module: not re-exported by the package entry. + */ + +import type { NavigationItem } from '@object-ui/types'; + +/** `usePermissions().can`, narrowed to the one action this gate asks about. */ +export type ObjectReadCheck = (object: string, action: 'read') => boolean; + +/** The one read verdict an object-bound entry or list action is drawn on. */ +export function mayReadObject(can: ObjectReadCheck, objectName: string): boolean { + return can(objectName, 'read'); +} + +/** + * `items` without the `type: 'object'` entries whose object the caller may not + * read, at every depth. Every other entry is kept as it is. + * + * Returns `items` itself when nothing is dropped, and a parent whose children + * are untouched keeps its identity, so a caller who may read everything hands + * the layout the very tree it was given. + */ +export function withoutUnreadableObjectEntries( + items: NavigationItem[], + can: ObjectReadCheck, +): NavigationItem[] { + let changed = false; + const kept: NavigationItem[] = []; + for (const item of items) { + if ( + item?.type === 'object' + && typeof item.objectName === 'string' + && item.objectName !== '' + && !mayReadObject(can, item.objectName) + ) { + changed = true; + continue; + } + const children = item?.type === 'separator' ? undefined : item?.children; + if (Array.isArray(children) && children.length > 0) { + const keptChildren = withoutUnreadableObjectEntries(children, can); + if (keptChildren !== children) { + changed = true; + kept.push({ ...item, children: keptChildren } as NavigationItem); + continue; + } + } + kept.push(item); + } + return changed ? kept : items; +} diff --git a/packages/app-shell/src/views/ObjectView.listActionsReadGate-12109.test.tsx b/packages/app-shell/src/views/ObjectView.listActionsReadGate-12109.test.tsx new file mode 100644 index 0000000000..c3d8f95a00 --- /dev/null +++ b/packages/app-shell/src/views/ObjectView.listActionsReadGate-12109.test.tsx @@ -0,0 +1,169 @@ +/** + * 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#12109 — an object's list actions are not drawn for a caller who may + * not read the object. + * + * Measured on HotCRM (objectstack-ai/hotcrm#2058, 17.7.0): a service agent with + * no read on `crm_opportunity` was shown "Update Stage" over the opportunity + * list's "You don't have access". That entry is not a navigation item: it is + * HotCRM's `MassUpdateStageAction` (`objectName: 'crm_opportunity'`, + * `locations: ['list_toolbar']`), drawn by THIS page's schema-driven toolbar. + * The fixture carries the keys of that declaration this toolbar reads. + * + * The verdict is the REAL `MePermissionsProvider`'s, holding the caller's + * `/me/permissions` answer, and it is the one the sidebar prunes the object's + * navigation entry on (`mayReadObject`), so an entry and its list actions go + * together. Every case is one differential: the same object, the same action, + * only the caller's `allowRead` differs. + * + * Harness: `ObjectView.objectBoundActions-7234.test.tsx`'s, except that + * `@object-ui/permissions` is NOT mocked. + */ + +import * as React from 'react'; +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { render, cleanup, screen, waitFor } from '@testing-library/react'; +import { MemoryRouter, Routes, Route } from 'react-router-dom'; +import { MePermissionsProvider, type MePermissionsResponse } from '@object-ui/permissions'; + +vi.mock('@object-ui/auth', async (importOriginal) => ({ + ...(await importOriginal>()), + useAuth: () => ({ user: { id: 'u_agent', name: 'Agent' }, activeOrganization: null }), + useWorkspaceAdminStatus: () => ({ isAdmin: false, isResolved: true }), + createAuthenticatedFetch: () => vi.fn(), +})); + +vi.mock('@object-ui/collaboration', async (importOriginal) => ({ + ...(await importOriginal>()), + useRealtimeSubscription: () => ({ lastMessage: null }), + useConflictResolution: () => ({ hasConflicts: false, resolveAllConflicts: () => {} }), +})); + +vi.mock('sonner', () => ({ + toast: Object.assign(vi.fn(), { + success: vi.fn(), + error: vi.fn(), + info: vi.fn(), + warning: vi.fn(), + loading: vi.fn(), + dismiss: vi.fn(), + }), +})); + +// Heavy children: orthogonal to the toolbar under test, as in the 7234 suite. +vi.mock('@object-ui/plugin-list', async (importOriginal) => ({ + ...(await importOriginal()), + ListView: () => null, +})); +vi.mock('@object-ui/plugin-view', async (importOriginal) => ({ + ...(await importOriginal>()), + ObjectView: () => null, + ViewTabBar: () => null, + ManageViewsDialog: () => null, +})); +vi.mock('./MetadataInspector', () => ({ + MetadataPanel: () => null, + useMetadataInspector: () => ({ showDebug: false, toggle: () => {} }), +})); +vi.mock('./RecordDetailView', () => ({ RecordDetailView: () => null })); + +import { ObjectView } from './ObjectView'; +import { ExpressionProvider } from '../providers/ExpressionProvider'; + +const OBJECT_NAME = 'crm_opportunity'; + +/** HotCRM's `MassUpdateStageAction`, abridged to the keys this toolbar reads. */ +const UPDATE_STAGE = { + name: 'mass_update_stage', + label: 'Update Stage', + objectName: OBJECT_NAME, + type: 'script', + locations: ['list_toolbar'], +}; + +const OBJECTS = [ + { + name: OBJECT_NAME, + label: 'Opportunity', + pluralLabel: 'Opportunities', + fields: { + id: { type: 'text', label: 'Id' }, + name: { type: 'text', label: 'Name' }, + }, + actions: [UPDATE_STAGE], + }, +]; + +function answerFor(opportunityRead: boolean): MePermissionsResponse { + return { + authenticated: true, + userId: 'u_agent', + tenantId: null, + roles: [], + permissionSets: ['service_agent'], + systemPermissions: [], + objects: { [OBJECT_NAME]: { allowRead: opportunityRead, allowEdit: true } }, + fields: {}, + }; +} + +function makeDataSource() { + return { + find: vi.fn(async () => ({ data: [], total: 0 })), + findOne: vi.fn(async () => null), + create: vi.fn(async () => ({})), + update: vi.fn(async () => ({})), + delete: vi.fn(async () => ({})), + } as never; +} + +const USER = { id: 'u_agent', name: 'Agent' }; + +function renderList(answer: MePermissionsResponse) { + return render( + + + + + {}} />} + /> + + + + , + ); +} + +const updateStage = () => screen.queryByRole('button', { name: /Update Stage/ }); + +/** Absence is only meaningful after the render has had a chance to settle. */ +async function settle() { + await new Promise((resolve) => setTimeout(resolve, 80)); +} + +describe('objectui#12109 — an object\'s list actions follow the caller\'s read on the object', () => { + afterEach(() => cleanup()); + + it('a caller who may read the object: "Update Stage" is drawn on the list toolbar', async () => { + renderList(answerFor(true)); + await waitFor(() => expect(updateStage()).toBeTruthy()); + }); + + it('the service agent, without read: the SAME action on the SAME page is not drawn', async () => { + renderList(answerFor(false)); + await settle(); + // Paired positive: the page itself rendered, so the absence below is the + // gate's and not an empty tree's. + expect(screen.getAllByText('Opportunities').length).toBeGreaterThan(0); + expect(updateStage()).toBeNull(); + }); +}); diff --git a/packages/app-shell/src/views/ObjectView.tsx b/packages/app-shell/src/views/ObjectView.tsx index 878bcee197..aee26d91aa 100644 --- a/packages/app-shell/src/views/ObjectView.tsx +++ b/packages/app-shell/src/views/ObjectView.tsx @@ -76,6 +76,7 @@ import { IdentityImportOptions, IdentityImportResultExtra, identityImportFields import { importTargetFields } from './importTargetFields.js'; import { useExpressionContext } from '../providers/ExpressionProvider.js'; import { listToolbarDrawsAction, resolveManagedByEmptyState } from '../utils/managedByEmptyState.js'; +import { mayReadObject } from '../utils/navObjectReadGate.js'; import { resolveViewId } from '../utils/resolveViewId.js'; import { defaultListViewId, viewRowId, isSavedViewId, viewEntry } from '../utils/viewIdentity.js'; import { warnSuppressedListNav } from '../utils/warnSuppressedListNav.js'; @@ -1993,11 +1994,21 @@ function ObjectViewInner({ dataSource, objects, onEdit, externalRefreshKey }: Co apiBase: (import.meta as any).env?.VITE_SERVER_URL || '', refreshKey, }); + // objectui#12109 — this object's list actions are drawn only for a caller + // who may read the object. Every one of them is bound to it (an action's + // `objectName` is what merges it into `objectDef.actions`), and a caller + // without read gets the list's "You don't have access" under them, so a + // drawn action could only end there. The verdict is the one the sidebar + // prunes this object's navigation entry on (`mayReadObject`), so the entry + // and its list actions go together. A boolean, read each render from the + // provider's verdict, so the memo below keys on a primitive (AGENTS.md #10). + const listActionsReadable = mayReadObject(perms.can, objectDef.name); + const listActions = listActionsReadable ? objectDef.actions : undefined; // Localized `list_toolbar` actions, shared by the generic action bar and the // environment-aware toolbar (the action:bar renderer filters by location). const localizedToolbarActions = useMemo( - () => (objectDef.actions || []).map((a: any) => localizeActionTexts(objectDef.name, a)), - [objectDef, localizeActionTexts], + () => (listActionsReadable ? objectDef.actions || [] : []).map((a: any) => localizeActionTexts(objectDef.name, a)), + [objectDef, localizeActionTexts, listActionsReadable], ); // [#5216] The DECLARED nav `runAction` slot, consumed generically. @@ -2127,7 +2138,7 @@ function ObjectViewInner({ dataSource, objects, onEdit, externalRefreshKey }: Co const toolbarPredicateScope = usePredicateScope(); const pageOffersCreate = (objectCanCreate && createVisible) || - listToolbarDrawsAction(objectDef.actions, mayInvokeToolbarAction, toolbarPredicateScope); + listToolbarDrawsAction(listActions, mayInvokeToolbarAction, toolbarPredicateScope); /** * [#5142] The object-list toolbar's IMPORT predicates — the `import` half @@ -3917,7 +3928,7 @@ function ObjectViewInner({ dataSource, objects, onEdit, externalRefreshKey }: Co )} {/* Schema-driven toolbar actions */} - {objectDef.actions?.some((a: any) => a.locations?.includes('list_toolbar')) && ( + {listActions?.some((a: any) => a.locations?.includes('list_toolbar')) && ( isEnvironmentList ? ( ) { + return { + apps: [app], + objects: [{ name: 'crm_account', label: 'Accounts' }, { name: 'crm_opportunity', label: 'Opportunities' }], + dashboards: [], + reports: [], + pages: [], + loading: false, + error: null, + refresh: async () => {}, + invalidate: () => {}, + ensureType: async () => [], + getItem: async () => null, + getItemsByType: () => [], + getTypeStatus: () => 'ready', + }; +} + +function renderMenu(answer: MePermissionsResponse, app: Record) { + render( + + + + + + } + /> + + + + , + ); + return within(screen.getByRole('navigation', { name: 'App navigation' })); +} + +describe('objectui#12109 — `nav:menu` prunes an unreadable object entry, as the sidebar does', () => { + it('the service agent: Opportunities and My Deals are not drawn; Accounts and the dashboard are', () => { + const menu = renderMenu(answerFor(false), { name: 'crm', label: 'CRM', navigation: NAVIGATION }); + expect(menu.queryByRole('link', { name: 'Opportunities' })).toBeNull(); + expect(menu.queryByRole('link', { name: 'My Deals' })).toBeNull(); + expect(menu.getByRole('link', { name: 'Accounts' })).toHaveAttribute('href', '/apps/crm/crm_account'); + // CONTROL: an entry not bound to an object is unchanged. + expect(menu.getByRole('link', { name: 'Home' })).toHaveAttribute('href', '/apps/crm/dashboard/crm_home'); + }); + + it('a caller who may read the object sees both entries', () => { + const menu = renderMenu(answerFor(true), { name: 'crm', label: 'CRM', navigation: NAVIGATION }); + expect(menu.getByRole('link', { name: 'Opportunities' })).toHaveAttribute('href', '/apps/crm/crm_opportunity'); + expect(menu.getByRole('link', { name: 'My Deals' })).toHaveAttribute('href', '/apps/crm/crm_opportunity/view/my_open_deals'); + }); + + it('the elected area is the first one with a READABLE entry', () => { + const menu = renderMenu(answerFor(false), { name: 'crm', label: 'CRM', navigation: [], areas: AREAS }); + expect(menu.getByRole('link', { name: 'Accounts' })).toBeInTheDocument(); + expect(menu.queryByRole('link', { name: 'Opportunities' })).toBeNull(); + }); +}); diff --git a/packages/app-shell/src/views/nav-menu-renderer.tsx b/packages/app-shell/src/views/nav-menu-renderer.tsx index 7dfb75c46e..4e58470c5c 100644 --- a/packages/app-shell/src/views/nav-menu-renderer.tsx +++ b/packages/app-shell/src/views/nav-menu-renderer.tsx @@ -48,6 +48,11 @@ * `requiresService` → a `doc` entry's member-readability answer), wired to * the same console providers and hook `UnifiedSidebar` wires them to. * + * The tree those guards run over is the sidebar's too: an entry bound to an + * object the caller may not read is pruned first, by the one function + * `UnifiedSidebar` prunes with (`withoutUnreadableObjectEntries`, in this + * package, objectui#12109) — the layout holds no permissions to ask. + * * ## Why not mount `NavigationRenderer` itself * * Measured, not assumed: `NavigationRenderer` renders through @@ -130,6 +135,7 @@ import { useExpressionContext, evaluateVisibility } from '../providers/Expressio import { useNavActionDispatch } from '../hooks/useNavActionDispatch.js'; import { useNavTargetLabel } from '../hooks/useNavTargetLabel.js'; import { useNavDocTargetCheck } from '../hooks/useNavDocTargetCheck.js'; +import { withoutUnreadableObjectEntries } from '../utils/navObjectReadGate.js'; import { useNavigationContext } from '../context/NavigationContext.js'; import { getIcon } from '../utils/getIcon.js'; import { appRouteSegment, matchAppBySegment } from '../utils/index.js'; @@ -235,16 +241,22 @@ export const NavMenuRenderer: React.FC = ({ [evalVis, checkPerm, checkCap, checkDocTarget], ); + // An entry bound to an object the caller may not read is not drawn + // (objectui#12109) — pruned before the guards run, through the same function + // `UnifiedSidebar` prunes with, so the area election and the drawn rows here + // read the tree the sidebar reads. const items: NavigationItem[] = useMemo(() => { + const readable = (navigation: NavigationItem[] | undefined): NavigationItem[] => + withoutUnreadableObjectEntries(navigation ?? [], can); const areas = (activeApp?.areas as any[]) || []; if (areas.length > 0) { const firstVisible = areas.find((area: any) => - hasVisibleNavigationItems(area?.navigation ?? [], guards), + hasVisibleNavigationItems(readable(area?.navigation), guards), ); - if (firstVisible) return firstVisible.navigation ?? []; + if (firstVisible) return readable(firstVisible.navigation); } - return (activeApp?.navigation as NavigationItem[]) ?? []; - }, [activeApp, guards]); + return readable(activeApp?.navigation as NavigationItem[] | undefined); + }, [activeApp, guards, can]); const basePath = activeApp ? `/apps/${appRouteSegment(activeApp) ?? activeApp.name}` : ''; diff --git a/packages/layout/src/NavigationRenderer.tsx b/packages/layout/src/NavigationRenderer.tsx index d9d86f3b4b..a27ce6f787 100644 --- a/packages/layout/src/NavigationRenderer.tsx +++ b/packages/layout/src/NavigationRenderer.tsx @@ -566,6 +566,14 @@ export interface NavigationVisibilityOptions { * A gate that holds on one path into a subtree and not on another is the * authoring trap this predicate exists to prevent, so the three callers share * the statement rather than agreeing about it. + * + * What it does NOT ask: whether the member may read the object a `type: + * 'object'` entry opens. This layer holds no permissions and none of its + * callbacks answers that question ({@link CapabilityChecker} asks whether the + * RUNTIME has the object), so a host that knows prunes such an entry from the + * tree before handing it here, which reaches all three callers at once. The + * console does (`withoutUnreadableObjectEntries` in `@object-ui/app-shell`, + * objectui#12109). */ function passesNavItemGuards( item: NavigationItem,