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
19 changes: 19 additions & 0 deletions .changeset/29630-user-preference-owner-fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
---
'@objectstack/plugin-auth': patch
---

fix(plugin-auth)!: a user preference's owner is fixed when the preference is saved

Clause-②: no (narrowing) — a non-system update of a `sys_user_preference` row that sends a `user_id` other than the stored one is refused `403 PERMISSION_DENIED`, which some callers' writes were admitted before. Nothing is widened.
Metadata-protocol: none

<!-- adr-0087: not-required (no-migration-prescription) No key of any schema is added, removed, renamed or re-shaped, and no export or type changes: a save rule on one platform object refuses a change of the row's owner. The refusal is a 403 a caller cannot work around by rewriting metadata, so there is nothing for a migration to rewrite and no tombstone. -->

**BREAKING** accept-set narrowing at the generic data door for `sys_user_preference` (`@objectstack/plugin-auth`).

- **Now.** A preference belongs to the user it was saved for. An update that sends a different `user_id` is refused `403 PERMISSION_DENIED`, for every caller but the system: a member, an organization administrator and a platform administrator alike. The refusal names the row and the `user_id` column. The row is unchanged.
- **Unchanged.** An update that echoes the stored `user_id`, or does not send it, saves as before. The console's preference writer updates `value` only and creates rows in the caller's own name, so it is not affected. A system write (the platform's own writers) is not judged.
- **Shared rule.** `sys_personal_view` and `sys_user_preference` now hold this with one check, `owner-fixed-rule.ts`. The personal view's behaviour and message are unchanged.
- **Who sees it.** A client that changes the `user_id` of a saved preference.

**The remedy.** Save a new preference in the other user's name, as that user, instead of moving one.
4 changes: 2 additions & 2 deletions content/docs/permissions/system-context.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ bugs — it is the protection or the side effect that silently does not happen.
| 25 | Engine-owned / append-only write guard bypassed | plugin-security | Get: generic writes to `managedBy` engine-owned objects | `packages/plugins/plugin-security/src/system-write-guard.ts#isUserContextWrite`, `packages/plugins/plugin-security/src/system-write-guard.ts#assertEngineOwnedWriteAllowed` |
| 26 | Identity write guard bypassed (ADR-0092) | plugin-auth | Get: direct writes to identity tables through the generic data path | `packages/plugins/plugin-auth/src/identity-write-guard.ts#isUserContextWrite` |
| 26b | API key restore guard does not refuse | plugin-auth | Get: a write setting `revoked: false` on a `sys_api_key` row an administrator's forced password reset revoked is admitted without the platform-administrator check a user-context restore must pass; the reason is still cleared. Lose: nothing a caller reaches: no system writer restores a key, and the mint, the last-used stamp and the forced reset itself never set `revoked: false` | `packages/plugins/plugin-auth/src/api-key-restore-guard.ts#isUserContextWrite` |
| 26c | **Personal-view owner rule does not refuse** — a `sys_personal_view` update that changes `user_id` is admitted | plugin-auth | Get: a system writer can move a personal view to another owner (no shipped system writer does). Lose: nothing a caller reaches. A non-system update whose `user_id` differs from the stored owner is refused `403 PERMISSION_DENIED`, for every caller, a platform administrator included; an echo of the stored owner is no change. The rule exists because, for an organization member, `member_default`'s `'*'` update floor (`created_by == current_user.id`) applies to an update in place of the object's own row scope | `packages/plugins/plugin-auth/src/personal-view-hooks.ts#registerPersonalViewHooks` |
| 26c | **Per-user owner rule does not refuse** — a `sys_personal_view` or `sys_user_preference` update that changes `user_id` is admitted | plugin-auth | Get: a system writer can move a personal view or a preference to another owner (no shipped system writer does). Lose: nothing a caller reaches. A non-system update whose `user_id` differs from the stored owner is refused `403 PERMISSION_DENIED`, for every caller, a platform administrator included; an echo of the stored owner is no change. One shared check holds it for both objects. The rule exists because, for an organization member, `member_default`'s `'*'` update floor (`created_by == current_user.id`) applies to an update beside the object's own row scope | `packages/plugins/plugin-auth/src/owner-fixed-rule.ts#isOwnerChange` |
| 27 | Search-companion column **kept** in a read's rows when it was explicitly requested | objectql | Get: the internal companion column is readable. Lose: nothing for app code — this is the engine reading its own index | `stripSearchCompanionFromRead` |
| 28 | Dependent-count disclosure on a blocked delete | objectql | Get: the count of blocking children. Nothing was elevated past the caller, so nothing is withheld | `dependentCountIsDisclosable` |
| 29 | Reference-cleanup log attributes the write to `'system'` | objectql | Get: an honest actor label instead of `anonymous` when the context carries neither `userId` nor `actor` | `recordReferenceCheckElevation` |
Expand Down Expand Up @@ -575,7 +575,7 @@ and CI refuses one that differs from what the tree produces.
| `packages/plugins/plugin-auth/src/api-key-restore-guard.ts#isUserContextWrite` | 1 | 26b |
| `packages/plugins/plugin-auth/src/auth-plugin.ts#start` | 1 | 12 |
| `packages/plugins/plugin-auth/src/identity-write-guard.ts#isUserContextWrite` | 1 | 26 |
| `packages/plugins/plugin-auth/src/personal-view-hooks.ts#registerPersonalViewHooks` | 1 | 26c |
| `packages/plugins/plugin-auth/src/owner-fixed-rule.ts#isOwnerChange` | 1 | 26c |
| `packages/plugins/plugin-auth/src/principal-session-mint.ts#refuseUpFront` | 1 | 61b |
| `packages/plugins/plugin-email/src/email-template-door.ts#isOrganizationWrite` | 1 | 59 |
| `packages/plugins/plugin-security/src/flow-system-identity-write-refusal.ts#elevationSource` | 1 | 1b |
Expand Down
7 changes: 7 additions & 0 deletions packages/plugins/plugin-auth/src/auth-plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ import {
import { registerLastAdminGuard } from './last-admin-guard.js';
import { registerApiKeyRestoreGuard } from './api-key-restore-guard.js';
import { registerPersonalViewHooks } from './personal-view-hooks.js';
import { registerUserPreferenceHooks } from './user-preference-hooks.js';
import { personalViewCallerResolver, registerPersonalViewRoutes, PERSONAL_VIEW_REMOVE_ROUTE, type PersonalViewEngine } from './personal-view-routes.js';
import { registerMembershipEndedSessionTrigger } from './membership-ended-session.js';
import { registerMemberRoleCanonicalization } from './member-role-canonical.js';
Expand Down Expand Up @@ -1872,6 +1873,12 @@ export class AuthPlugin implements Plugin {
packageId: 'com.objectstack.plugin-auth.personal-view',
logger: ctx.logger,
});
// A user's own preferences: the owner (user_id) is fixed when the row
// is saved, for every caller but the system. See user-preference-hooks.ts.
registerUserPreferenceHooks(engine, {
packageId: 'com.objectstack.plugin-auth.user-preference',
logger: ctx.logger,
});
// [cloud ADR-0024 D5.2] Break-glass — the same identity write
// chokepoints, guarding a different question: not "may this caller
// write identity tables" (above, and system writes bypass it by
Expand Down
6 changes: 4 additions & 2 deletions packages/plugins/plugin-auth/src/identity-objects-plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ import { resolveTenancyPosture } from '@objectstack/types';
import { ensureDefaultOrganizationExists } from './default-organization-invariant.js';
import { authIdentityManifest } from './manifest.js';
import { registerPersonalViewHooks } from './personal-view-hooks.js';
import { registerUserPreferenceHooks } from './user-preference-hooks.js';

/** The kernel plugin name of {@link IdentityObjectsPlugin}. */
export const IDENTITY_OBJECTS_PLUGIN_NAME = 'com.objectstack.auth.identity-objects';
Expand Down Expand Up @@ -123,11 +124,12 @@ export class IdentityObjectsPlugin implements Plugin {
* plugin keeps it. A failure throws and fails the boot, never warn-only.
*/
async start(ctx: PluginContext): Promise<void> {
// The manifest above registers `sys_personal_view`; its two write rules
// travel with it, exactly as `AuthPlugin` registers them.
// The manifest above registers `sys_personal_view` and `sys_user_preference`;
// their write rules travel with them, exactly as `AuthPlugin` registers them.
const engine = ctx.getService<{ registerHook?: unknown }>('objectql');
if (engine && typeof engine.registerHook === 'function') {
registerPersonalViewHooks(engine, { packageId: `${IDENTITY_OBJECTS_PLUGIN_NAME}.personal-view` });
registerUserPreferenceHooks(engine, { packageId: `${IDENTITY_OBJECTS_PLUGIN_NAME}.user-preference` });
}
if (postureInForce(ctx) !== 'single') return;
const invariant = await ensureDefaultOrganizationExists(ctx.getService<IDataEngine>('objectql'), {
Expand Down
71 changes: 71 additions & 0 deletions packages/plugins/plugin-auth/src/owner-fixed-rule.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* **The owner of a per-user row is fixed when the row is saved.** The one
* spelling of that rule, shared by `sys_personal_view`
* (`personal-view-hooks.ts`) and `sys_user_preference`
* (`user-preference-hooks.ts`), whose owner column is `user_id`.
*
* An update that sends an owner other than the stored one is refused
* `403 PERMISSION_DENIED`, for every caller but the system: a member, an
* organization administrator and a platform administrator alike. An echo of
* the stored owner is no change, and saves.
*
* A save rule is the one place that judges the CHANGE of owner; why the row
* scope alone does not is row 26c of `content/docs/permissions/system-context.mdx`.
*
* The refusal names the row and the owner column, and never the id the write
* sent.
*/

/** The string form two stored ids are compared by (an echo of the stored owner is no change). */
function sameId(a: unknown, b: unknown): boolean {
const norm = (v: unknown) => (v === undefined || v === null || v === '' ? null : String(v));
return norm(a) === norm(b);
}

const sets = (data: unknown, key: string): boolean =>
!!data && typeof data === 'object' && Object.prototype.hasOwnProperty.call(data, key);

/**
* Whether a `beforeUpdate` hook context is a non-system write handing a row to
* another owner: it sends `ownerField` with a value other than the stored one
* (`ctx.previous`, present on the by-id path and per row on a multi-row
* update). A system write is never one: the platform's own writers are
* trusted to mean it.
*/
export function isOwnerChange(ctx: any, ownerField = 'user_id'): boolean {
if (ctx?.session?.isSystem === true) return false;
const data = ctx?.input?.data;
const previous = ctx?.previous;
if (!sets(data, ownerField) || !previous || typeof previous !== 'object') return false;
return !sameId((data as Record<string, unknown>)[ownerField], (previous as Record<string, unknown>)[ownerField]);
}

/** How a refusal names the row it guards. */
export interface OwnerFixedSubject {
/** The row's kind as a sentence starts it: `Personal view`, `User preference`. */
label: string;
/** The row's kind inside a sentence: `view`, `preference`. */
noun: string;
}

/** The refusal for an update that would hand a row to another user: `PERMISSION_DENIED` / `403`. */
export function ownerFixedError(subject: OwnerFixedSubject, id: unknown, ownerField = 'user_id'): Error {
const { label, noun } = subject;
const err = new Error(
`${label} ${String(id ?? '')} was not changed: a ${noun}'s owner (${ownerField}) is fixed when the ${noun} is saved, `
+ `and nobody can hand a ${noun} to another user. Save a new ${noun} in that user's name instead. `
+ `The ${noun} is unchanged.`,
) as Error & { code?: string; status?: number };
err.code = 'PERMISSION_DENIED';
err.status = 403;
return err;
}

/** Throw {@link ownerFixedError} when {@link isOwnerChange} holds for `ctx`. */
export function assertOwnerUnchanged(ctx: any, subject: OwnerFixedSubject, ownerField = 'user_id'): void {
if (isOwnerChange(ctx, ownerField)) {
throw ownerFixedError(subject, ctx?.previous?.id ?? ctx?.input?.id, ownerField);
}
}
35 changes: 6 additions & 29 deletions packages/plugins/plugin-auth/src/personal-view-hooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,8 @@
*
* **The owner is fixed when the view is saved.** An update that sends a
* `user_id` other than the stored one is refused `403 PERMISSION_DENIED`, for
* every caller but the system. The row scope alone does not hold this: for an
* organization member, `member_default`'s `'*'` update floor
* (`created_by == current_user.id`) is a held update policy, so the object's
* declared default does not apply to an update, and the post-image of a
* re-parent still carries the member's own `created_by`.
* every caller but the system. The rule, and why the row scope alone does not
* hold it, is `owner-fixed-rule.ts`'s, shared with `sys_user_preference`.
*
* Which view opens by default is not this object's: the reader's existing
* per-object pin holds a view id (seat ruling 6587007982 on #29443).
Expand All @@ -37,6 +34,7 @@
import { ListViewSchema } from '@objectstack/spec/ui';
import { zodIssuesToFields } from '@objectstack/spec/api';
import { validationFailure } from '@objectstack/types';
import { assertOwnerUnchanged } from './owner-fixed-rule.js';

/** The object these hooks guard. */
export const PERSONAL_VIEW_OBJECT = 'sys_personal_view';
Expand Down Expand Up @@ -95,22 +93,8 @@ export function assertPersonalViewConfig(config: unknown, objectName?: unknown):
);
}

/** The string form two stored ids are compared by (an echo of the stored owner is no change). */
function sameId(a: unknown, b: unknown): boolean {
const norm = (v: unknown) => (v === undefined || v === null || v === '' ? null : String(v));
return norm(a) === norm(b);
}

/** The refusal for an update that would hand a view to another user: `PERMISSION_DENIED` / `403`. */
export function ownerChangeError(id: unknown): Error {
const err = new Error(
`Personal view ${String(id ?? '')} was not changed: a view's owner (user_id) is fixed when the view is saved, `
+ 'and nobody can hand a view to another user. Save a new view in that user\'s name instead. The view is unchanged.',
) as Error & { code?: string; status?: number };
err.code = 'PERMISSION_DENIED';
err.status = 403;
return err;
}
/** How the shared owner rule names this object's rows. */
const PERSONAL_VIEW_SUBJECT = { label: 'Personal view', noun: 'view' } as const;

const sets = (data: Record<string, unknown> | null | undefined, key: string): boolean =>
!!data && typeof data === 'object' && Object.prototype.hasOwnProperty.call(data, key);
Expand All @@ -133,14 +117,7 @@ export function registerPersonalViewHooks(engine: any, opts: PersonalViewHooksOp
// The row as it will be: what the write sends, over what is stored
// (`ctx.previous`, present on the by-id path and per row on a multi-row update).
const previous = (ctx.previous ?? undefined) as Record<string, unknown> | undefined;
if (
ctx?.session?.isSystem !== true
&& sets(data, 'user_id')
&& previous
&& !sameId(data!.user_id, previous.user_id)
) {
throw ownerChangeError(previous.id ?? ctx.input?.id);
}
assertOwnerUnchanged(ctx, PERSONAL_VIEW_SUBJECT);
const sendsConfig = sets(data, CONFIG_FIELD);
if (!sendsConfig && !sets(data, 'object_name')) return;
const config = sendsConfig ? data![CONFIG_FIELD] : previous?.[CONFIG_FIELD];
Expand Down
Loading
Loading