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
47 changes: 47 additions & 0 deletions .changeset/sharing-write-denial-localized.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
---
"@objectstack/plugin-sharing": minor
---

fix(plugin-sharing): the by-id write denial renders through the Operation
Message Catalog instead of a hardcoded English sentence (#12260, the consumer
half of the key #12493 landed)

A user holding object-level allowRead + allowEdit — and no `modifyAllRecords` —
PATCHed a record they do not own on an object declaring
`sharingModel: 'public_read'` with `access: { default: 'private' }`. The sharing
middleware refused, correctly, and the client showed the server's reason
verbatim to the end user: one hardcoded English sentence naming the object's API
name and the row's opaque id. In a fully Chinese deployment that was the only
thing the user was told about why their save failed.

The refusal now renders through the shared Operation Message Catalog in
`@objectstack/spec/system` under the key `record_write_denied` that #12493
landed for it — the same mechanism `plugin-security`'s record-level denial
already uses, which is exactly the comparison the report drew: the same "I can
see this record but cannot change it" situation showed human language or raw
English depending on which layer refused. Same resolution ladder (deployment
override → the caller's locale → `en` → the key), same guarantee that a
misbehaving i18n service cannot turn a 403 into a 500. All four platform
locales (`en`, `zh-CN`, `ja-JP`, `es-ES`) ship copy that sends the reader to the
record's owner or an administrator instead of dead-ending them.

`record_write_denied` is deliberately not `record_access_denied`: this gate
fires on a row the READ path already admitted, so "You do not have access to
this record" would be false the moment it rendered. It is one key for BOTH write
verbs — the user's situation and remedy are identical for `update` and `delete`.

`buildSharingMiddleware` gains an optional third argument, a lazily resolved
`II18nService.t`-compatible lookup wired by `SharingServicePlugin`, because the
i18n service is contributed by another plugin and may start later. It is what
makes the override address the catalog documents,
`errors.record_write_denied`, take effect for this emitter. The argument is
additive: every existing caller passes two and is unchanged, and a stack with no
i18n service still renders the built-in catalog in the caller's locale.

**Not changed: who may write.** The gate is byte-identical — ownership, write
depth, an edit-level share for `update`, Modify All Data — and the app-authored
RLS deferral ahead of it is untouched. The `FORBIDDEN:` prefix the REST layer
classifies 403 on is untouched, and so is the ADR-0111 D10 `delete`-verb
diagnostic breadcrumb. The verb, object and row id the old sentence carried are
now developer facts on the error's `developerMessage` and `details` and in the
log, where a developer reads them and a user never does.
Original file line numberDiff line numberDiff line change
Expand Up@@ -11,6 +11,13 @@
// middleware's, not the row-gate's `(row-level security)` — so the refusal
// landed BEFORE RLS was consulted and the declared widener was never asked.
//
// [#12260] That English sentence is HISTORY as of this card: the refusal's
// user-facing half now renders from the Operation Message Catalog
// (`record_write_denied`) and the verb/object/id it used to name moved to
// `developerMessage`. The tell is unchanged in substance — `[sharing] …` vs
// the row gate's `(row-level security)` — only its channel moved. See
// `write-denial-user-copy.test.ts`.
//
// The discriminator is not "carries sharing rules" (#5493's own wording) but
// **whether record sharing enforces on the object at all** (round-2 refinement,
// issue comment 5226364929): `checkEdit` abstains — and `canEdit` therefore
Expand DownExpand Up@@ -52,6 +59,7 @@
// and that everything that is not a literal `admit` leaves the refusal intact.
import { describe, it, expect, beforeEach, vi } from 'vitest';
import { assertEngineDeleteDispatch, assertEngineUpdateDispatch, assertEngineFindOnePredicate } from '@objectstack/objectql';
import { BUILTIN_OPERATION_MESSAGES } from '@objectstack/spec/system';
import { SharingService, type SharingSecurityProbe } from './sharing-service.js';
import { buildSharingMiddleware } from './sharing-plugin.js';

Expand DownExpand Up@@ -311,6 +319,8 @@ interface WriteOutcome {
code?: string;
status?: number;
message: string;
/** [#12260] The developer half the user-facing sentence no longer carries. */
developerMessage?: string;
}

interface Stack {
Expand DownExpand Up@@ -376,7 +386,10 @@ function makeStack(opts: {
reached = true;
});
} catch (e: any) {
return { ok: false, code: e?.code, status: e?.status, message: String(e?.message ?? e) };
return {
ok: false, code: e?.code, status: e?.status,
message: String(e?.message ?? e), developerMessage: e?.developerMessage,
};
}
return reached
? { ok: true, message: 'written' }
Expand DownExpand Up@@ -405,7 +418,15 @@ function expectSharingRefusal(out: WriteOutcome, operation: 'update' | 'delete',
expect(out.ok, `expected a refusal, got a completed ${operation}`).toBe(false);
expect(out.code, 'ADR-0112 error code').toBe('FORBIDDEN');
expect(out.status, 'ADR-0112 HTTP status').toBe(403);
expect(out.message).toContain(`FORBIDDEN: insufficient privileges to ${operation} ${object} ${id}`);
// [#12260] The SENTENCE moved onto the Operation Message Catalog (key
// `record_write_denied`), so the discriminator this file turns on moved with
// it: the verb, the object's API name and the row id are now developer copy.
// Both halves are asserted, because both are how this refusal is told apart
// from `plugin-security`'s row gate — which answers `PERMISSION_DENIED` with
// its own `(row-level security)` breadcrumb and never writes `[sharing]`.
// The `FORBIDDEN:` prefix is wire contract and is unchanged.
expect(out.message).toBe(`FORBIDDEN: ${BUILTIN_OPERATION_MESSAGES.en.record_write_denied}`);
expect(out.developerMessage).toContain(`[sharing] ${operation} denied on ${object} ${id}`);
}

const rowById = (stack: Stack, object: string, id: string) =>
Expand Down
111 changes: 109 additions & 2 deletions packages/plugins/plugin-sharing/src/sharing-plugin.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -22,6 +22,12 @@ import type { ExecutionContext } from '@objectstack/spec/kernel';
// mark `plugin-security` and `service-analytics` stamp at theirs — never a
// local flag, never a second spelling of the same idea.
import { markFilterSubtreeProvenance } from '@objectstack/spec/data';
// [#12260] The SANCTIONED renderer for OPERATION-level refusal copy. The
// Operation Message Catalog is the ONE seat for these sentences — its own
// header bars both a package-local string table and a second rendering
// mechanism for a second producer, and #12493 landed this middleware's key
// (`record_write_denied`) into it ahead of this consumer half.
import { renderOperationMessage, type ValidationMessageTranslator } from '@objectstack/spec/system';
import { SysRecordShare, SysSharingRule, SysShareLink } from './objects/index.js';
import { SysBusinessUnit, SysBusinessUnitMember } from '@objectstack/platform-objects/identity';
import {
Expand DownExpand Up@@ -610,7 +616,7 @@ export class SharingServicePlugin implements Plugin {
if (this.options.enforce === false) {
ctx.logger.info('SharingServicePlugin: enforcement disabled (enforce=false) — share-link service still registered');
} else {
const mw = buildSharingMiddleware(this.service, ctx.logger as any);
const mw = buildSharingMiddleware(this.service, ctx.logger as any, pluginMessageTranslator(ctx));
if (typeof engine.registerMiddleware === 'function') {
engine.registerMiddleware(mw, { object: '*' });
ctx.logger.info('SharingServicePlugin: enforcement middleware installed');
Expand DownExpand Up@@ -954,6 +960,73 @@ export class SharingServicePlugin implements Plugin {
}
}

/**
* [#12260] The END USER's half of the by-id write refusal.
*
* This middleware's refusal declares `{ code: 'FORBIDDEN', status: 403 }`, so
* `@objectstack/rest` answers it through the DECLARED-status arm and ships
* `error.message` to the client as the body's human-readable `error` — which
* Console renders verbatim in a toast. One hardcoded English sentence
* therefore reached a business user in a fully Chinese deployment as the only
* thing they were told about why their save failed.
*
* Rendered through the SHARED Operation Message Catalog
* (`@objectstack/spec/system`), not a second mechanism: same `errors.<key>`
* override address, same resolution ladder (deployment override -> locale
* catalog -> `en` -> the key), same guarantee that a misbehaving i18n service
* cannot turn a 403 into a 500. `plugin-security`'s `userFacingDenialMessage`
* is the sibling consumer this mirrors, and `plugin-approvals`'
* `userFacingRefusal` (#11993) is the same conversion one card earlier.
*
* ⛔ ONE key for BOTH write verbs, which is the catalog's own ruling and not a
* shortcut taken here: the user's situation (they can see this record, they
* cannot change it) and their remedy (ask its owner, or an administrator) are
* identical for `update` and `delete`. WHICH verb was refused is a developer
* fact and stays on `developerMessage`, on the structured `details`, and — for
* `delete` — on the ADR-0111 D10 breadcrumb that keeps its own wording.
*
* The `FORBIDDEN:` prefix is NOT part of what this renders. It is wire
* contract (ADR-0111's `CODE: message` idiom, which the share routes read and
* strip) and it is applied by the caller around this sentence.
*
* The translator is resolved LAZILY, per refusal, for the reason ADR-0029 D8
* makes structural: the i18n service is contributed by a different plugin that
* may start after this one, so a lookup captured when the middleware was built
* would pin `undefined` for the life of the process. Absent is a SUPPORTED
* stack, not a degraded one — the built-in catalog still renders the caller's
* locale; what the translator adds is the documented override address
* `errors.record_write_denied`.
*/
function userFacingWriteDenial(
locale: string | undefined,
messageTranslator?: () => ValidationMessageTranslator | undefined,
): string {
let translate: ValidationMessageTranslator | undefined;
try {
translate = messageTranslator?.();
} catch {
// i18n is optional and late-bound; the built-in catalog still renders the
// caller's locale without it.
translate = undefined;
}
return renderOperationMessage({ messageKey: 'record_write_denied' }, { locale, translate });
}

/**
* [#12260] The deployment i18n lookup this plugin hands its middleware, read
* through `PluginContext` on every refusal rather than captured at start().
* See {@link userFacingWriteDenial} for why late binding is the requirement
* and not a defensive habit.
*/
function pluginMessageTranslator(ctx: PluginContext): () => ValidationMessageTranslator | undefined {
return () => {
const i18n = ctx.getService<II18nService>('i18n');
const t = i18n?.t;
if (typeof t !== 'function') return undefined;
return (key: string, loc: string, params?: Record<string, unknown>) => t.call(i18n, key, loc, params);
};
}

/**
* Build the engine middleware that injects read filters and gates
* write operations. Exported so it can be unit-tested without booting
Expand All@@ -971,6 +1044,15 @@ export class SharingServicePlugin implements Plugin {
export function buildSharingMiddleware(
service: SharingService,
log?: { warn?: (msg: string, meta?: any) => void },
/**
* [#12260] Deployment i18n lookup for the by-id write refusal's user-facing
* sentence — an `II18nService.t`-compatible function, resolved LAZILY per
* refusal. Optional and additive: every existing caller (six suites in
* `plugin-security`, two here) passes two arguments and is unchanged, and a
* stack without it still renders the caller's locale from the built-in
* catalog. See {@link userFacingWriteDenial}.
*/
messageTranslator?: () => ValidationMessageTranslator | undefined,
): EngineMiddleware {
return async function sharingMiddleware(ctx: OperationContext, next: () => Promise<void>) {
const op = ctx.operation;
Expand DownExpand Up@@ -1116,11 +1198,36 @@ export function buildSharingMiddleware(
{ object: ctx.object, recordId: String(id), userId: exec?.userId },
);
}
// [#12260] The DEVELOPER's half — the verb, the object's API name and
// the row id. This USED TO BE the whole message, which is how it
// reached an end user's toast in English; the catalog sentence
// deliberately names none of it (the only spellings available here
// are an API name and an opaque id, the #7414 vocabulary that must
// not reach a toast). It is kept where a developer reads it and a
// user never does: on the error, and in the log line below. REST
// ships neither `developerMessage` nor `details` on a FORBIDDEN
// body — only `DELETE_RESTRICTED` forwards a `developerMessage` —
// so this adds nothing to the wire.
const developerMessage =
`[sharing] ${verb} denied on ${ctx.object} ${id}: the caller holds no ${verb} authority ` +
`over this row (owner match, share depth and Modify All Data all answered no)`;
log?.warn?.(developerMessage, {
object: ctx.object,
recordId: String(id),
operation: verb,
userId: exec?.userId,
});
// The `FORBIDDEN:` PREFIX STAYS. It is not user copy — it is the
// ADR-0111 `CODE: message` idiom the share routes read and strip,
// and it sits beside the `code`/`status` the `/data` door
// classifies on. Only the SENTENCE after it moved.
const err: any = new Error(
`FORBIDDEN: insufficient privileges to ${op} ${ctx.object} ${id}`,
`FORBIDDEN: ${userFacingWriteDenial(exec?.locale, messageTranslator)}`,
);
err.code = 'FORBIDDEN';
err.status = 403;
err.developerMessage = developerMessage;
err.details = { operation: verb, object: ctx.object, recordId: String(id) };
throw err;
}
return next();
Expand Down
Loading
Loading