Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions .changeset/26190-delegation-scope-refusal-reason.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
---
'@object-ui/react': patch
'@object-ui/components': patch
'@object-ui/app-shell': patch
---

fix(react,components,app-shell): a delegated admin's out-of-scope save shows the server's reason instead of the fixed no-permission line

Clause-②: yes (widening: `@object-ui/react` exports a new `delegationScopeMessage`)
Metadata-protocol: none

When a delegated administrator saved something outside their own scope, the server answered `403 DELEGATION_SCOPE_VIOLATION` with the boundary that refused it (a business unit outside the delegated subtree, a permission set not in the scope's allowlist, a target user outside the subtree). The console read every 403 as a plain denial, so the toast and the in-form banner said only "You don't have permission to save this record."

- **`@object-ui/react`.** New `delegationScopeMessage(err)`: the server's reason, without the `[Security] ` machine prefix, when the error's code (`code` or `details.code`) is `DELEGATION_SCOPE_VIOLATION`; `null` for everything else. It is read before `isPermissionError`, which still answers true for any 403.
- **`@object-ui/components`.** The record form's toast and banner show that reason. A producer-marked `userMessage` still comes first.
- **`@object-ui/app-shell`.** A flow run whose write node met the same refusal (`innerRefusal`) shows the same reason.

Every other denial is unchanged: `PERMISSION_DENIED`, `FORBIDDEN`, a bare 403 and a row-level-security refusal still show the fixed localized line and never the server's text.

What to do: nothing.
19 changes: 19 additions & 0 deletions ui/packages/app-shell/src/utils/__tests__/flowResponse.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -696,6 +696,25 @@ describe('interpretFlowResponse — a refusal met inside the run, by the record-
expect((zh as { error: string }).error).not.toContain('crm_account');
});

// objectstack#26059 Q3: a delegated admin's out-of-scope write is the one
// denial whose reason is shown; it describes only the caller's own scope.
it('a delegated-admin scope refusal shows the server reason, not the fixed permission line', () => {
const SCOPE_REASON =
"Outside your delegated scope: delegated 'create' on sys_user_position rejected — business unit 'bu_west' is outside the delegated subtree (scope from 'regional_admin').";
const SCOPE = { code: 'DELEGATION_SCOPE_VIOLATION', message: `[Security] ${SCOPE_REASON}` } as const;

expect(failed400(flowFailed({ innerRefusal: SCOPE }))).toEqual({
kind: 'failed', error: SCOPE_REASON, retryable: false,
});
const on200 = interpretFlowResponse(ok, {
success: true,
data: { success: false, status: 'failed', error: RAW_ERROR, innerRefusal: SCOPE },
}, 'Flow "assign"');
expect(on200).toMatchObject({ kind: 'failed', error: SCOPE_REASON });
// Control: a plain denial on the same path still reads the fixed line.
expect(failed400(flowFailed({ innerRefusal: DENIED }))).toMatchObject({ error: builtInLocales.en.form.noPermissionToSave });
});

it('a producer-marked userMessage wins over the inner refusal, verbatim', () => {
expect(failed400(flowFailed({ innerRefusal: UNIQUE }, USER_TEXT))).toMatchObject({ error: USER_TEXT });
expect(failed400(flowFailed({ innerRefusal: DENIED }, USER_TEXT))).toMatchObject({ error: USER_TEXT });
Expand Down
8 changes: 6 additions & 2 deletions ui/packages/app-shell/src/utils/flowResponse.ts
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@
import { actionCopySource, actionErrorDetail } from '@object-ui/core';
import type { ActionResult } from '@object-ui/core';
import { getLoadedBuiltInLocales } from '@object-ui/i18n';
import { declaredUserMessage, isPermissionError } from '@object-ui/react';
import { declaredUserMessage, delegationScopeMessage, isPermissionError } from '@object-ui/react';
import { errorCodeIs } from '@object-ui/types';
import type { AutomationResult } from '@objectstack/spec/contracts';
import { translateFlow, type TranslationBundle } from '@objectstack/spec/system';
Expand Down Expand Up @@ -368,7 +368,9 @@ function permissionDeniedLine(locale: string | undefined): string {
* The inner refusal is rendered exactly as the record form renders the same
* refusal on a save: a permission denial (`isPermissionError`) becomes the
* form's localized permission line instead of server prose (objectstack#3821),
* anything else reads its own sentence. `message` stays the last rung, so a
* anything else reads its own sentence. A delegated-admin scope refusal
* (`DELEGATION_SCOPE_VIOLATION`) reads its own reason, asked before the
* permission check exactly as the form asks it (objectstack#26059, Q3). `message` stays the last rung, so a
* run that met no refusal still says something, never nothing.
*/
function flowFailureMessage(failure: FlowFailureCarriers, message: string, copy: FlowCopyLocale | undefined): string {
Expand All @@ -378,6 +380,8 @@ function flowFailureMessage(failure: FlowFailureCarriers, message: string, copy:
const inner = failure.innerRefusal as { code?: unknown; message?: unknown } | null | undefined;
if (inner && typeof inner === 'object' && typeof inner.code === 'string') {
const refusal = { code: inner.code, message: typeof inner.message === 'string' ? inner.message : '' };
const scopeReason = delegationScopeMessage(refusal);
if (scopeReason) return scopeReason;
if (isPermissionError(refusal)) return permissionDeniedLine(copy?.locale);
if (refusal.message.trim()) return refusal.message;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,3 +160,53 @@ describe('a producer-marked refusal reaches the user', () => {
expect(await screen.findByText(/title must be unique/i)).toBeInTheDocument();
});
});

/**
* objectstack#26059 Q3 (child #26190) — a delegated administrator who saves
* outside their own scope is told which boundary refused it.
*
* `403 DELEGATION_SCOPE_VIOLATION` is the one denial whose server reason is
* shown: it describes only the caller's own delegated scope. Every other denial
* keeps the fixed line (objectstack#24914 B′, unchanged), which the plain-403
* control below holds.
*/
describe('a delegated-admin scope refusal shows the server reason', () => {
const REASON =
"Outside your delegated scope: delegated 'update' on sys_user_position rejected — business unit 'bu_west' is outside the delegated subtree (scope from 'regional_admin').";

beforeEach(() => {
vi.spyOn(console, 'warn').mockImplementation(() => {});
});

function scopeRefusal() {
// The shape `@objectstack/client` hands the form for a 403: the server's
// message on `message`, the semantic code on `code`, the status on
// `httpStatus`.
return Object.assign(new Error(`[Security] ${REASON}`), {
code: 'DELEGATION_SCOPE_VIOLATION',
httpStatus: 403,
});
}

it("renders the server's reason, without the machine prefix, instead of the fixed line", async () => {
const { container } = renderForm(async () => { throw scopeRefusal(); });
fireEvent.click(screen.getByRole("button", { name: /submit/i }));

expect(await screen.findByText(REASON)).toBeInTheDocument();
expect(screen.queryByText(/permission to save/i)).not.toBeInTheDocument();
expect(container.textContent).not.toMatch(/\[Security\]/);
});

it('CONTROL: a plain 403 PERMISSION_DENIED keeps the fixed line and withholds its text', async () => {
const { container } = renderForm(async () => {
throw Object.assign(
new Error("[Security] Access denied: delegated 'update' on sys_user_position rejected — no delegated scope covers this write."),
{ code: 'PERMISSION_DENIED', httpStatus: 403 },
);
});
fireEvent.click(screen.getByRole("button", { name: /submit/i }));

expect(await screen.findByText(/permission to save/i)).toBeInTheDocument();
expect(container.textContent).not.toMatch(/sys_user_position/);
});
});
9 changes: 8 additions & 1 deletion ui/packages/components/src/renderers/form/form.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ import { FullscreenEditor } from '../../custom/fullscreen-editor';
import { CharacterCount } from '../../custom/character-count';
import { cn } from '../../lib/utils';
import React from 'react';
import { SchemaRendererContext, usePredicateScope, isPermissionError, extractWriteErrorMessage, extractFieldErrors, declaredUserMessage } from '@object-ui/react';
import { SchemaRendererContext, usePredicateScope, isPermissionError, extractWriteErrorMessage, extractFieldErrors, declaredUserMessage, delegationScopeMessage } from '@object-ui/react';
import { createSafeTranslation } from '@object-ui/i18n';

/**
Expand Down Expand Up @@ -3048,8 +3048,15 @@ ComponentRegistry.register('form',
// construction rather than by us guessing what a 403 body contains.
// Status-agnostic on purpose: 403 is where this was reported
// (objectui#5210), not a fence the contract draws.
// A delegated administrator's out-of-scope write (`403
// DELEGATION_SCOPE_VIOLATION`) is the one denial whose reason is shown:
// it describes only the caller's own scope, and it tells them which
// boundary to fix (objectstack#26059, Q3). It is read before
// `isPermissionError`, which answers true for any 403; every other
// denial keeps the fixed line (objectstack#24914 B′).
const errorMessage =
declaredUserMessage(error) ??
delegationScopeMessage(error) ??
(isPermissionError(error)
? t('form.noPermissionToSave')
: extractWriteErrorMessage(error) ?? t('form.submitFailed'));
Expand Down
2 changes: 1 addition & 1 deletion ui/packages/react/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ export type { PredicateGateKind } from './utils/visibilityDiagnostic.js';

// Write-error surfacing utilities (shared by drag-write plugins so a failed
// PATCH — e.g. an RLS 403 — is never silently swallowed).
export { extractWriteErrorMessage, isPermissionError, extractFieldErrors, classifyLoadError, declaredUserMessage } from './utils/error-message.js';
export { extractWriteErrorMessage, isPermissionError, extractFieldErrors, classifyLoadError, declaredUserMessage, delegationScopeMessage } from './utils/error-message.js';
export type { WriteFieldError, LoadErrorKind } from './utils/error-message.js';

// Built-in i18n support
Expand Down
43 changes: 42 additions & 1 deletion ui/packages/react/src/utils/error-message.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { describe, it, expect } from 'vitest';

import { ApiErrorSchema } from '@objectstack/spec/api';

import { extractFieldErrors, classifyLoadError, declaredUserMessage } from './error-message';
import { extractFieldErrors, classifyLoadError, declaredUserMessage, delegationScopeMessage, isPermissionError } from './error-message';

describe('extractFieldErrors', () => {
// The wire shape the server actually sends: `@objectstack/rest` serves a
Expand Down Expand Up @@ -250,3 +250,44 @@ describe('declaredUserMessage', () => {
expect(declaredUserMessage(parsedUnmarked)).toBeNull();
});
});

// objectstack#26059 Q3: the one denial whose server reason a surface shows.
describe('delegationScopeMessage', () => {
const REASON =
"Outside your delegated scope: delegated 'update' on sys_user_permission_set rejected — permission set 'finance_admin' is not in the scope's allowlist (scope from 'regional_admin').";

it("answers the server's reason, without the machine prefix, for a 403 DELEGATION_SCOPE_VIOLATION", () => {
const err = Object.assign(new Error(`[Security] ${REASON}`), {
code: 'DELEGATION_SCOPE_VIOLATION',
httpStatus: 403,
});
expect(delegationScopeMessage(err)).toBe(REASON);
// The reason it must be asked first: the permission predicate claims it too.
expect(isPermissionError(err)).toBe(true);
});

it('reads the code from `details` too, the pair isPermissionError reads', () => {
const err = Object.assign(new Error(`[Security] ${REASON}`), {
httpStatus: 403,
details: { code: 'DELEGATION_SCOPE_VIOLATION' },
});
expect(delegationScopeMessage(err)).toBe(REASON);
});

it('CONTROL: every other denial answers null, so its text stays withheld', () => {
const denied = (code: string | undefined) =>
Object.assign(new Error("[Security] Access denied: delegated 'update' on sys_user_position rejected — no scope."), {
code,
httpStatus: 403,
});
expect(delegationScopeMessage(denied('PERMISSION_DENIED'))).toBeNull();
expect(delegationScopeMessage(denied('FORBIDDEN'))).toBeNull();
expect(delegationScopeMessage(denied(undefined))).toBeNull();
expect(delegationScopeMessage(new Error('row-level security policy violation'))).toBeNull();
expect(delegationScopeMessage(null)).toBeNull();
});

it('answers null for a scope refusal with no usable text, so the caller still says something', () => {
expect(delegationScopeMessage({ code: 'DELEGATION_SCOPE_VIOLATION', httpStatus: 403, message: '' })).toBeNull();
});
});
42 changes: 42 additions & 0 deletions ui/packages/react/src/utils/error-message.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
*/

import { isApiAccessDeniedError } from '@object-ui/core';
import { errorCodeIs } from '@object-ui/types';
import type { ApiError } from '@objectstack/spec/api';

/**
Expand Down Expand Up @@ -136,6 +137,43 @@ export function declaredUserMessage(err: unknown): string | null {
return null;
}

/**
* The ADR-0112 code a delegated administrator's write answers when it falls
* outside the caller's OWN `adminScope` (the ledger row registered under
* `@objectstack/plugin-security`, thrown as `DelegationScopeViolationError`).
*/
const DELEGATION_SCOPE_VIOLATION = 'DELEGATION_SCOPE_VIOLATION';

/**
* The server's reason for a delegated-admin scope refusal, or `null` when the
* refusal is anything else.
*
* A `403 DELEGATION_SCOPE_VIOLATION` names which boundary of the caller's own
* delegated scope refused the write (a business unit outside the delegated
* subtree, a permission set outside the allowlist, a target user outside the
* subtree). That text describes only the caller's own scope, so it is safe to
* show, and it tells the delegate what to fix (ruling on objectstack#26059, Q3).
* Every other denial stays opaque: `PERMISSION_DENIED`, `FORBIDDEN`, a bare 403
* or a row-level-security refusal answers `null` here, and callers keep their
* fixed localized line (ruling B′ on objectstack#24914, unchanged).
*
* Callers must ask this BEFORE {@link isPermissionError}: that predicate answers
* true for any 403, so a scope refusal read after it is flattened into the
* plain denial. The code is read from the same pair {@link isPermissionError}
* reads (`code`, then `details.code`); the text is
* {@link extractWriteErrorMessage}'s, without the `[Security] ` machine prefix.
* A scope refusal that carries no usable text answers `null`, so the caller
* still says something.
*/
export function delegationScopeMessage(err: unknown): string | null {
if (!err || typeof err !== 'object') return null;
const details = (err as { details?: unknown }).details;
if (!errorCodeIs(err, DELEGATION_SCOPE_VIOLATION) && !errorCodeIs(details, DELEGATION_SCOPE_VIOLATION)) {
return null;
}
return extractWriteErrorMessage(err);
}

/** One field the server rejected, normalised for `form.setError`. */
export interface WriteFieldError {
/** Field name, matching the form control's `name`. */
Expand Down Expand Up @@ -198,6 +236,10 @@ export function extractFieldErrors(err: unknown): WriteFieldError[] | null {
* `PERMISSION_DENIED` / `FORBIDDEN` / a row-level-security denial). Lets a
* caller show a friendly "you don't have permission" toast rather than dumping
* the raw server text.
*
* A delegated-admin scope refusal is a 403 too, so this answers true for it:
* a caller that shows that refusal's reason asks
* {@link delegationScopeMessage} first.
*/
export function isPermissionError(err: unknown): boolean {
const e = err as Record<string, any> | null | undefined;
Expand Down
Loading