From e6f17916f6f1982889ad3d5c3deca09d61b7b76d Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 23:12:52 +0000 Subject: [PATCH 1/3] fix(plugin-auth): a user preference's owner is fixed when it is saved One shared owner-fixed rule (owner-fixed-rule.ts) now holds the check sys_personal_view carried inline, and sys_user_preference registers it too: a non-system update that sends a user_id other than the stored one is refused 403 PERMISSION_DENIED, for every caller but the system. Claude-Session: https://claude.ai/code/session_01ADHkmTNFJm8TGKvyoyKegL Co-authored-by: Claude --- .../plugins/plugin-auth/src/auth-plugin.ts | 7 + .../src/identity-objects-plugin.ts | 6 +- .../plugin-auth/src/owner-fixed-rule.ts | 78 ++++++++ .../plugin-auth/src/personal-view-hooks.ts | 35 +--- .../src/user-preference-hooks.test.ts | 189 ++++++++++++++++++ .../plugin-auth/src/user-preference-hooks.ts | 49 +++++ 6 files changed, 333 insertions(+), 31 deletions(-) create mode 100644 packages/plugins/plugin-auth/src/owner-fixed-rule.ts create mode 100644 packages/plugins/plugin-auth/src/user-preference-hooks.test.ts create mode 100644 packages/plugins/plugin-auth/src/user-preference-hooks.ts diff --git a/packages/plugins/plugin-auth/src/auth-plugin.ts b/packages/plugins/plugin-auth/src/auth-plugin.ts index 5f2df8bd3e1..3964b585fb4 100644 --- a/packages/plugins/plugin-auth/src/auth-plugin.ts +++ b/packages/plugins/plugin-auth/src/auth-plugin.ts @@ -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'; @@ -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 diff --git a/packages/plugins/plugin-auth/src/identity-objects-plugin.ts b/packages/plugins/plugin-auth/src/identity-objects-plugin.ts index fc219f2622e..a03b2594357 100644 --- a/packages/plugins/plugin-auth/src/identity-objects-plugin.ts +++ b/packages/plugins/plugin-auth/src/identity-objects-plugin.ts @@ -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'; @@ -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 { - // 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('objectql'), { diff --git a/packages/plugins/plugin-auth/src/owner-fixed-rule.ts b/packages/plugins/plugin-auth/src/owner-fixed-rule.ts new file mode 100644 index 00000000000..bc860c1e0ec --- /dev/null +++ b/packages/plugins/plugin-auth/src/owner-fixed-rule.ts @@ -0,0 +1,78 @@ +// 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 every per-user object whose owner column is + * `user_id` rather than the engine-stamped `created_by`: `sys_personal_view` + * (`personal-view-hooks.ts`) and `sys_user_preference` + * (`user-preference-hooks.ts`). + * + * 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. + * + * 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 for every object, and the row policies held for one + * object and operation are OR-combined: the pre-image and the post-image of a + * re-parent both still carry the member's own `created_by`, so the floor + * admits a write that moves the row out of the owner's `user_id` scope. An + * administrator whose set opens every row of the object is bound by no row + * policy at all. A save rule is the one place that judges the CHANGE. + * + * 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)[ownerField], (previous as Record)[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); + } +} diff --git a/packages/plugins/plugin-auth/src/personal-view-hooks.ts b/packages/plugins/plugin-auth/src/personal-view-hooks.ts index 42e3039fef0..03e07feaaab 100644 --- a/packages/plugins/plugin-auth/src/personal-view-hooks.ts +++ b/packages/plugins/plugin-auth/src/personal-view-hooks.ts @@ -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). @@ -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'; @@ -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 | null | undefined, key: string): boolean => !!data && typeof data === 'object' && Object.prototype.hasOwnProperty.call(data, key); @@ -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 | 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]; diff --git a/packages/plugins/plugin-auth/src/user-preference-hooks.test.ts b/packages/plugins/plugin-auth/src/user-preference-hooks.test.ts new file mode 100644 index 00000000000..dc32d1eea1c --- /dev/null +++ b/packages/plugins/plugin-auth/src/user-preference-hooks.test.ts @@ -0,0 +1,189 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `sys_user_preference`'s write rule — the owner (`user_id`) is fixed when the + * preference is saved — driven through the real write path: a real `ObjectQL` + * engine over SQLite, the real `SecurityPlugin` with the shipped permission + * sets, and the hooks `AuthPlugin` registers for the object + * (`user-preference-hooks.ts`, on the shared `owner-fixed-rule.ts`). + * + * What each block pins: + * - an organization member, whose `member_default` update floor is in force, + * cannot hand their own preference to another member; + * - an organization administrator and a platform administrator are refused + * the same way, though each can otherwise update a member's preference; + * - controls: an update that echoes the stored owner saves, the owner's own + * reads and writes are unchanged, and a system write is not judged. + * + * Refusals are asserted on the ADR-0112 pair (`code` + `status`), on the row + * left unchanged, and on a message that names the row and the owner column and + * echoes no other user's id. + */ + +import { describe, it, expect, afterEach, vi } from 'vitest'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { SecurityPlugin } from '@objectstack/plugin-security'; + +import { authIdentityObjects } from './manifest.js'; +import { USER_PREFERENCE_OBJECT, registerUserPreferenceHooks } from './user-preference-hooks.js'; + +const SYS = { context: { isSystem: true } } as any; +const ORG = 'org_default'; +const ALICE = 'usr_alice'; +const BOB = 'usr_bob'; +const CAROL = 'usr_carol'; +const OBJ = USER_PREFERENCE_OBJECT; + +/** A caller as the transports hand one to the engine; the baseline set resolves from the user id. */ +const as = (userId: string, permissions: string[] = [], positions: string[] = ['org_member']) => + ({ context: { userId, tenantId: ORG, positions, permissions } }) as any; + +const engines: ObjectQL[] = []; +afterEach(async () => { + vi.restoreAllMocks(); + while (engines.length) { + try { await engines.pop()?.destroy(); } catch { /* noop */ } + } +}); + +/** Engine + shipped permission sets + the hook `auth-plugin.ts` registers for the object. */ +async function rig(opts: { hooks?: boolean } = {}): Promise { + const engine = new ObjectQL(); + engine.registerDriver( + new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true }), + true, + ); + await engine.init(); + engines.push(engine); + let manifest: { objects?: unknown[]; permissions?: unknown[] } = {}; + const services: Record = { + manifest: { register: (m: any) => { manifest = m; } }, + objectql: engine, + metadata: { + get: async (type: string, name: string) => + type === 'permission' + ? ((manifest.permissions as any[]) ?? []).find((p) => p?.name === name) ?? null + : engine.getSchema(name) ?? null, + list: async (type?: string) => + type === undefined || type === 'permission' ? [...((manifest.permissions as unknown[]) ?? [])] : [], + }, + }; + const ctx: any = { + logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }, + hook: vi.fn(), + registerService: vi.fn(), + getService: (name: string) => { + if (!(name in services)) throw new Error(`service not registered: ${name}`); + return services[name]; + }, + }; + const security = new SecurityPlugin({ fallbackPermissionSet: 'member_default' }); + await security.init(ctx); + engine.registerApp({ + id: 'com.objectstack.qa.user-preference', + name: 'User preferences', + version: '1.0.0', + type: 'plugin', + scope: 'system', + objects: [ + ...authIdentityObjects, + ...((manifest.objects ?? []) as any[]).filter((o) => + ['sys_position', 'sys_user_position', 'sys_position_permission_set', 'sys_user_permission_set'].includes(o?.name)), + ], + } as any); + await engine.syncSchemas(); + await engine.insert('sys_organization', { id: ORG, name: 'Default Organization', slug: 'default' }, SYS); + await security.start(ctx); + vi.spyOn((engine as any).logger, 'warn').mockImplementation(() => undefined); + if (opts.hooks !== false) registerUserPreferenceHooks(engine, { packageId: 'test.user-preference' }); + for (const id of [ALICE, BOB, CAROL]) { + await engine.insert('sys_user', { id, email: `${id}@prefs.example`, name: id, email_verified: true, banned: false }, SYS); + } + return engine; +} + +const refusalOf = async (write: Promise): Promise => write.then(() => undefined, (e: unknown) => e); + +/** One stored row, read as the system (what is actually on disk). */ +const storedRow = async (engine: ObjectQL, id: string): Promise => + engine.findOne(OBJ, { where: { id }, ...SYS }); + +async function save(engine: ObjectQL, owner: string, key: string, value: unknown = ['a']): Promise { + return engine.insert(OBJ, { user_id: owner, key, value }, as(owner)); +} + +describe('a preference\'s owner is fixed when it is saved', () => { + it('a member cannot hand their own preference to another member: 403, and the row keeps its owner', async () => { + const engine = await rig(); + const own = await save(engine, ALICE, 'ui.recent'); + const err = await refusalOf(engine.update(OBJ, { id: own.id, user_id: BOB }, as(ALICE))); + expect({ code: err?.code, status: err?.status }).toEqual({ code: 'PERMISSION_DENIED', status: 403 }); + expect((await storedRow(engine, own.id))?.user_id).toBe(ALICE); + expect((await engine.find(OBJ, as(BOB))) as any[]).toEqual([]); + }); + + it('the refusal names the row and the owner column, and echoes no other user\'s id', async () => { + const engine = await rig(); + const own = await save(engine, ALICE, 'ui.recent'); + const err = await refusalOf(engine.update(OBJ, { id: own.id, user_id: BOB, value: ['b'] }, as(ALICE))); + expect(err?.status).toBe(403); + expect(String(err?.message)).toContain(String(own.id)); + expect(String(err?.message)).toContain('user_id'); + expect(String(err?.message)).not.toContain(BOB); + expect((await storedRow(engine, own.id))?.value).toEqual(['a']); + }); + + it.each([ + ['an organization administrator', 'organization_admin'], + ['a platform administrator', 'admin_full_access'], + ])('%s cannot hand a member\'s preference to someone else either', async (_who, adminSet) => { + const engine = await rig(); + const own = await save(engine, ALICE, 'ui.recent'); + const err = await refusalOf(engine.update(OBJ, { id: own.id, user_id: BOB }, as(CAROL, [adminSet], []))); + expect({ code: err?.code, status: err?.status }).toEqual({ code: 'PERMISSION_DENIED', status: 403 }); + expect((await storedRow(engine, own.id))?.user_id).toBe(ALICE); + }); + + it('a multi-row update is judged per row: a predicate write moving the owner is refused, and no row moves', async () => { + const engine = await rig(); + const own = await save(engine, ALICE, 'ui.recent'); + const err = await refusalOf( + engine.update(OBJ, { user_id: BOB }, { where: { key: 'ui.recent' }, multi: true, ...as(ALICE) } as any), + ); + expect({ code: err?.code, status: err?.status }).toEqual({ code: 'PERMISSION_DENIED', status: 403 }); + expect((await storedRow(engine, own.id))?.user_id).toBe(ALICE); + }); +}); + +describe('controls: everything else a preference write does is unchanged', () => { + it('an update that echoes the stored owner is not a re-parent, and saves', async () => { + const engine = await rig(); + const own = await save(engine, ALICE, 'ui.recent'); + await engine.update(OBJ, { id: own.id, user_id: ALICE, value: ['b'] }, as(ALICE)); + const row = await storedRow(engine, own.id); + expect({ user_id: row?.user_id, value: row?.value }).toEqual({ user_id: ALICE, value: ['b'] }); + }); + + it('the owner reads, lists and updates their own preferences', async () => { + const engine = await rig(); + const own = await save(engine, ALICE, 'ui.recent'); + await engine.update(OBJ, { id: own.id, value: ['c'] }, as(ALICE)); + const mine = (await engine.find(OBJ, as(ALICE))) as any[]; + expect(mine.map((r) => [r.key, r.value])).toEqual([['ui.recent', ['c']]]); + }); + + it('an organization administrator still updates a member\'s preference value', async () => { + const engine = await rig(); + const own = await save(engine, ALICE, 'ui.recent'); + await engine.update(OBJ, { id: own.id, value: ['by the admin'] }, as(CAROL, ['organization_admin'], [])); + expect((await storedRow(engine, own.id))?.value).toEqual(['by the admin']); + }); + + it('a system write is not judged: the platform\'s own writers may move a preference', async () => { + const engine = await rig(); + const own = await save(engine, ALICE, 'ui.recent'); + await engine.update(OBJ, { id: own.id, user_id: BOB }, SYS); + expect((await storedRow(engine, own.id))?.user_id).toBe(BOB); + }); +}); diff --git a/packages/plugins/plugin-auth/src/user-preference-hooks.ts b/packages/plugins/plugin-auth/src/user-preference-hooks.ts new file mode 100644 index 00000000000..e7e4a7da931 --- /dev/null +++ b/packages/plugins/plugin-auth/src/user-preference-hooks.ts @@ -0,0 +1,49 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The write rule of `sys_user_preference`, a user's own per-user key-value + * state (pinned list views, recent items, the console's remembered UI state). + * + * **The owner is fixed when the preference 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: a member, an organization administrator and a + * platform administrator alike. An update that echoes the stored owner saves. + * The rule, and why the object's row scope alone does not hold it, is + * `owner-fixed-rule.ts`'s, shared with `sys_personal_view`. + * + * What it leaves alone: a create is the row scope's (a preference created in + * another member's name is refused there); the console's preference writer + * (`userState.ts` in `@object-ui/data-objectstack`) updates `value` only and + * creates in the caller's own name; a system write is never judged, and no + * shipped system writer moves a preference. + * + * Registered by every plugin that registers the object (`AuthPlugin`, and + * `IdentityObjectsPlugin` on a kernel without it), so the rule travels with the + * table. + */ + +import { assertOwnerUnchanged } from './owner-fixed-rule.js'; + +/** The object these hooks guard. */ +export const USER_PREFERENCE_OBJECT = 'sys_user_preference'; + +/** How the shared owner rule names this object's rows. */ +const USER_PREFERENCE_SUBJECT = { label: 'User preference', noun: 'preference' } as const; + +export interface UserPreferenceHooksOptions { + packageId: string; + logger?: { info(msg: string): void }; +} + +/** + * Register the rule on an ObjectQL engine. Re-binding after hot reload + * follows the engine's standard contract: `engine.unregisterHooksByPackage(packageId)` first. + */ +export function registerUserPreferenceHooks(engine: any, opts: UserPreferenceHooksOptions): void { + const ownerIsFixed = async (ctx: any): Promise => { + if (ctx?.object !== USER_PREFERENCE_OBJECT) return; + assertOwnerUnchanged(ctx, USER_PREFERENCE_SUBJECT); + }; + engine.registerHook('beforeUpdate', ownerIsFixed, { object: USER_PREFERENCE_OBJECT, packageId: opts.packageId }); + opts.logger?.info('[UserPreferenceHooks] sys_user_preference hook registered: a preference\'s owner never changes'); +} From 3bbd5da8463b9a100b8991ec6b6bab822204c703 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 23:24:31 +0000 Subject: [PATCH 2/3] test(plugin-auth): pin the preference owner rule on the hook and over REST The rule is judged on the hook for every non-system caller, and over the real showcase stack for a member, an organization administrator and a platform administrator, with the echo and own-row controls. The system-context census row covers both per-user objects on the shared check, and the changeset records the narrowing. Claude-Session: https://claude.ai/code/session_01ADHkmTNFJm8TGKvyoyKegL Co-authored-by: Claude --- .../29630-user-preference-owner-fixed.md | 19 ++ content/docs/permissions/system-context.mdx | 4 +- .../plugin-auth/src/owner-fixed-rule.ts | 11 +- .../src/user-preference-hooks.test.ts | 214 ++++++------------ ...ser-preference-owner-fixed.dogfood.test.ts | 142 ++++++++++++ 5 files changed, 232 insertions(+), 158 deletions(-) create mode 100644 .changeset/29630-user-preference-owner-fixed.md create mode 100644 packages/qa/dogfood/test/user-preference-owner-fixed.dogfood.test.ts diff --git a/.changeset/29630-user-preference-owner-fixed.md b/.changeset/29630-user-preference-owner-fixed.md new file mode 100644 index 00000000000..4267e305564 --- /dev/null +++ b/.changeset/29630-user-preference-owner-fixed.md @@ -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 + + + +**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 moved a preference row to another user through `PATCH /api/v1/data/sys_user_preference/:id`. + +**The remedy.** Save a new preference in the other user's name, as that user, instead of moving one. diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index aad4fa7dae8..3b0a6648af6 100644 --- a/content/docs/permissions/system-context.mdx +++ b/content/docs/permissions/system-context.mdx @@ -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` | @@ -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 | diff --git a/packages/plugins/plugin-auth/src/owner-fixed-rule.ts b/packages/plugins/plugin-auth/src/owner-fixed-rule.ts index bc860c1e0ec..3de88998f77 100644 --- a/packages/plugins/plugin-auth/src/owner-fixed-rule.ts +++ b/packages/plugins/plugin-auth/src/owner-fixed-rule.ts @@ -12,14 +12,11 @@ * organization administrator and a platform administrator alike. An echo of * the stored owner is no change, and saves. * - * The row scope alone does not hold this. For an organization member, + * 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 for every object, and the row policies held for one - * object and operation are OR-combined: the pre-image and the post-image of a - * re-parent both still carry the member's own `created_by`, so the floor - * admits a write that moves the row out of the owner's `user_id` scope. An - * administrator whose set opens every row of the object is bound by no row - * policy at all. A save rule is the one place that judges the CHANGE. + * held update policy for every object, beside the object's own row scope, and + * the post-image of a re-parent still carries the member's own `created_by`. + * A save rule is the one place that judges the CHANGE of owner. * * The refusal names the row and the owner column, and never the id the write * sent. diff --git a/packages/plugins/plugin-auth/src/user-preference-hooks.test.ts b/packages/plugins/plugin-auth/src/user-preference-hooks.test.ts index dc32d1eea1c..ed6e8cc3683 100644 --- a/packages/plugins/plugin-auth/src/user-preference-hooks.test.ts +++ b/packages/plugins/plugin-auth/src/user-preference-hooks.test.ts @@ -2,188 +2,104 @@ /** * `sys_user_preference`'s write rule — the owner (`user_id`) is fixed when the - * preference is saved — driven through the real write path: a real `ObjectQL` - * engine over SQLite, the real `SecurityPlugin` with the shipped permission - * sets, and the hooks `AuthPlugin` registers for the object - * (`user-preference-hooks.ts`, on the shared `owner-fixed-rule.ts`). + * preference is saved — judged on the hook `AuthPlugin` and + * `IdentityObjectsPlugin` register (`user-preference-hooks.ts`, on the shared + * `owner-fixed-rule.ts`), with the hook context the engine hands it. * - * What each block pins: - * - an organization member, whose `member_default` update floor is in force, - * cannot hand their own preference to another member; - * - an organization administrator and a platform administrator are refused - * the same way, though each can otherwise update a member's preference; - * - controls: an update that echoes the stored owner saves, the owner's own - * reads and writes are unchanged, and a system write is not judged. + * The rule is held on its own here, for every non-system caller whatever its + * permission sets: which callers reach the hook over REST, and what the row + * scope answers first, is the HTTP face's + * (`packages/qa/dogfood/test/user-preference-owner-fixed.dogfood.test.ts`). * - * Refusals are asserted on the ADR-0112 pair (`code` + `status`), on the row - * left unchanged, and on a message that names the row and the owner column and - * echoes no other user's id. + * Refusals are asserted on the ADR-0112 pair (`code` + `status`) and on a + * message that names the row and the owner column and echoes no other user's + * id. */ -import { describe, it, expect, afterEach, vi } from 'vitest'; -import { ObjectQL } from '@objectstack/objectql'; -import { SqlDriver } from '@objectstack/driver-sql'; -import { SecurityPlugin } from '@objectstack/plugin-security'; +import { describe, it, expect } from 'vitest'; -import { authIdentityObjects } from './manifest.js'; import { USER_PREFERENCE_OBJECT, registerUserPreferenceHooks } from './user-preference-hooks.js'; -const SYS = { context: { isSystem: true } } as any; -const ORG = 'org_default'; -const ALICE = 'usr_alice'; -const BOB = 'usr_bob'; -const CAROL = 'usr_carol'; -const OBJ = USER_PREFERENCE_OBJECT; - -/** A caller as the transports hand one to the engine; the baseline set resolves from the user id. */ -const as = (userId: string, permissions: string[] = [], positions: string[] = ['org_member']) => - ({ context: { userId, tenantId: ORG, positions, permissions } }) as any; - -const engines: ObjectQL[] = []; -afterEach(async () => { - vi.restoreAllMocks(); - while (engines.length) { - try { await engines.pop()?.destroy(); } catch { /* noop */ } - } -}); +const ALICE = 'usr_owner_placeholder'; +const BOB = 'usr_other_placeholder'; +const ROW = 'pref_row_placeholder'; + +type Hook = (ctx: any) => Promise; -/** Engine + shipped permission sets + the hook `auth-plugin.ts` registers for the object. */ -async function rig(opts: { hooks?: boolean } = {}): Promise { - const engine = new ObjectQL(); - engine.registerDriver( - new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true }), - true, +/** The hooks the registrar binds, by event, with the options it bound them under. */ +function registered(): { hooks: Record; options: any[] } { + const hooks: Record = {}; + const options: any[] = []; + registerUserPreferenceHooks( + { registerHook: (event: string, fn: Hook, opts: any) => { (hooks[event] ??= []).push(fn); options.push({ event, ...opts }); } }, + { packageId: 'test.user-preference' }, ); - await engine.init(); - engines.push(engine); - let manifest: { objects?: unknown[]; permissions?: unknown[] } = {}; - const services: Record = { - manifest: { register: (m: any) => { manifest = m; } }, - objectql: engine, - metadata: { - get: async (type: string, name: string) => - type === 'permission' - ? ((manifest.permissions as any[]) ?? []).find((p) => p?.name === name) ?? null - : engine.getSchema(name) ?? null, - list: async (type?: string) => - type === undefined || type === 'permission' ? [...((manifest.permissions as unknown[]) ?? [])] : [], - }, - }; - const ctx: any = { - logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }, - hook: vi.fn(), - registerService: vi.fn(), - getService: (name: string) => { - if (!(name in services)) throw new Error(`service not registered: ${name}`); - return services[name]; - }, - }; - const security = new SecurityPlugin({ fallbackPermissionSet: 'member_default' }); - await security.init(ctx); - engine.registerApp({ - id: 'com.objectstack.qa.user-preference', - name: 'User preferences', - version: '1.0.0', - type: 'plugin', - scope: 'system', - objects: [ - ...authIdentityObjects, - ...((manifest.objects ?? []) as any[]).filter((o) => - ['sys_position', 'sys_user_position', 'sys_position_permission_set', 'sys_user_permission_set'].includes(o?.name)), - ], - } as any); - await engine.syncSchemas(); - await engine.insert('sys_organization', { id: ORG, name: 'Default Organization', slug: 'default' }, SYS); - await security.start(ctx); - vi.spyOn((engine as any).logger, 'warn').mockImplementation(() => undefined); - if (opts.hooks !== false) registerUserPreferenceHooks(engine, { packageId: 'test.user-preference' }); - for (const id of [ALICE, BOB, CAROL]) { - await engine.insert('sys_user', { id, email: `${id}@prefs.example`, name: id, email_verified: true, banned: false }, SYS); - } - return engine; + return { hooks, options }; } -const refusalOf = async (write: Promise): Promise => write.then(() => undefined, (e: unknown) => e); +/** A `beforeUpdate` context as the engine builds one on the by-id path. */ +const updateCtx = (data: Record, session: Record = { userId: ALICE }) => ({ + object: USER_PREFERENCE_OBJECT, + input: { id: ROW, data }, + previous: { id: ROW, user_id: ALICE, key: 'ui.recent', value: ['a'] }, + session, +}); -/** One stored row, read as the system (what is actually on disk). */ -const storedRow = async (engine: ObjectQL, id: string): Promise => - engine.findOne(OBJ, { where: { id }, ...SYS }); +const refusalOf = async (run: Promise): Promise => run.then(() => undefined, (e: unknown) => e); -async function save(engine: ObjectQL, owner: string, key: string, value: unknown = ['a']): Promise { - return engine.insert(OBJ, { user_id: owner, key, value }, as(owner)); -} +describe('registration', () => { + it('binds one beforeUpdate hook on sys_user_preference, and nothing on create or delete', () => { + const { hooks, options } = registered(); + expect(Object.keys(hooks)).toEqual(['beforeUpdate']); + expect(options).toEqual([{ event: 'beforeUpdate', object: USER_PREFERENCE_OBJECT, packageId: 'test.user-preference' }]); + }); +}); describe('a preference\'s owner is fixed when it is saved', () => { - it('a member cannot hand their own preference to another member: 403, and the row keeps its owner', async () => { - const engine = await rig(); - const own = await save(engine, ALICE, 'ui.recent'); - const err = await refusalOf(engine.update(OBJ, { id: own.id, user_id: BOB }, as(ALICE))); + it.each([ + ['an organization member', { userId: ALICE, positions: ['org_member'] }], + ['an organization administrator', { userId: 'usr_org_admin_placeholder', permissions: ['organization_admin'] }], + ['a platform administrator', { userId: 'usr_platform_admin_placeholder', permissions: ['admin_full_access'] }], + ])('%s cannot hand a preference to another user: 403 PERMISSION_DENIED', async (_who, session) => { + const [hook] = registered().hooks.beforeUpdate; + const err = await refusalOf(hook(updateCtx({ user_id: BOB }, session))); expect({ code: err?.code, status: err?.status }).toEqual({ code: 'PERMISSION_DENIED', status: 403 }); - expect((await storedRow(engine, own.id))?.user_id).toBe(ALICE); - expect((await engine.find(OBJ, as(BOB))) as any[]).toEqual([]); }); it('the refusal names the row and the owner column, and echoes no other user\'s id', async () => { - const engine = await rig(); - const own = await save(engine, ALICE, 'ui.recent'); - const err = await refusalOf(engine.update(OBJ, { id: own.id, user_id: BOB, value: ['b'] }, as(ALICE))); + const [hook] = registered().hooks.beforeUpdate; + const err = await refusalOf(hook(updateCtx({ user_id: BOB, value: ['b'] }))); expect(err?.status).toBe(403); - expect(String(err?.message)).toContain(String(own.id)); + expect(String(err?.message)).toContain(ROW); expect(String(err?.message)).toContain('user_id'); expect(String(err?.message)).not.toContain(BOB); - expect((await storedRow(engine, own.id))?.value).toEqual(['a']); }); - it.each([ - ['an organization administrator', 'organization_admin'], - ['a platform administrator', 'admin_full_access'], - ])('%s cannot hand a member\'s preference to someone else either', async (_who, adminSet) => { - const engine = await rig(); - const own = await save(engine, ALICE, 'ui.recent'); - const err = await refusalOf(engine.update(OBJ, { id: own.id, user_id: BOB }, as(CAROL, [adminSet], []))); - expect({ code: err?.code, status: err?.status }).toEqual({ code: 'PERMISSION_DENIED', status: 403 }); - expect((await storedRow(engine, own.id))?.user_id).toBe(ALICE); - }); - - it('a multi-row update is judged per row: a predicate write moving the owner is refused, and no row moves', async () => { - const engine = await rig(); - const own = await save(engine, ALICE, 'ui.recent'); - const err = await refusalOf( - engine.update(OBJ, { user_id: BOB }, { where: { key: 'ui.recent' }, multi: true, ...as(ALICE) } as any), - ); + it('clearing the owner is a change of owner too', async () => { + const [hook] = registered().hooks.beforeUpdate; + const err = await refusalOf(hook(updateCtx({ user_id: null }))); expect({ code: err?.code, status: err?.status }).toEqual({ code: 'PERMISSION_DENIED', status: 403 }); - expect((await storedRow(engine, own.id))?.user_id).toBe(ALICE); }); }); -describe('controls: everything else a preference write does is unchanged', () => { - it('an update that echoes the stored owner is not a re-parent, and saves', async () => { - const engine = await rig(); - const own = await save(engine, ALICE, 'ui.recent'); - await engine.update(OBJ, { id: own.id, user_id: ALICE, value: ['b'] }, as(ALICE)); - const row = await storedRow(engine, own.id); - expect({ user_id: row?.user_id, value: row?.value }).toEqual({ user_id: ALICE, value: ['b'] }); +describe('controls: what the rule leaves alone', () => { + it('an update that echoes the stored owner is no change', async () => { + const [hook] = registered().hooks.beforeUpdate; + await expect(hook(updateCtx({ user_id: ALICE, value: ['b'] }))).resolves.toBeUndefined(); }); - it('the owner reads, lists and updates their own preferences', async () => { - const engine = await rig(); - const own = await save(engine, ALICE, 'ui.recent'); - await engine.update(OBJ, { id: own.id, value: ['c'] }, as(ALICE)); - const mine = (await engine.find(OBJ, as(ALICE))) as any[]; - expect(mine.map((r) => [r.key, r.value])).toEqual([['ui.recent', ['c']]]); + it('an update that does not send the owner is not judged', async () => { + const [hook] = registered().hooks.beforeUpdate; + await expect(hook(updateCtx({ value: ['b'] }))).resolves.toBeUndefined(); }); - it('an organization administrator still updates a member\'s preference value', async () => { - const engine = await rig(); - const own = await save(engine, ALICE, 'ui.recent'); - await engine.update(OBJ, { id: own.id, value: ['by the admin'] }, as(CAROL, ['organization_admin'], [])); - expect((await storedRow(engine, own.id))?.value).toEqual(['by the admin']); + it('a system write is not judged: the platform\'s own writers may move a preference', async () => { + const [hook] = registered().hooks.beforeUpdate; + await expect(hook(updateCtx({ user_id: BOB }, { isSystem: true }))).resolves.toBeUndefined(); }); - it('a system write is not judged: the platform\'s own writers may move a preference', async () => { - const engine = await rig(); - const own = await save(engine, ALICE, 'ui.recent'); - await engine.update(OBJ, { id: own.id, user_id: BOB }, SYS); - expect((await storedRow(engine, own.id))?.user_id).toBe(BOB); + it('another object is not this rule\'s', async () => { + const [hook] = registered().hooks.beforeUpdate; + await expect(hook({ ...updateCtx({ user_id: BOB }), object: 'sys_other' })).resolves.toBeUndefined(); }); }); diff --git a/packages/qa/dogfood/test/user-preference-owner-fixed.dogfood.test.ts b/packages/qa/dogfood/test/user-preference-owner-fixed.dogfood.test.ts new file mode 100644 index 00000000000..c1fbaa22d33 --- /dev/null +++ b/packages/qa/dogfood/test/user-preference-owner-fixed.dogfood.test.ts @@ -0,0 +1,142 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `sys_user_preference` over REST on the real showcase stack: a preference's + * owner (`user_id`) is fixed when it is saved. The rule is plugin-auth's + * (`user-preference-hooks.ts`, on the shared `owner-fixed-rule.ts`, whose + * engine-level pins live beside it); this file is the HTTP face a console + * reader actually reaches, with the real composition, the real authorization + * resolver and the real error mapping: + * + * - `[member]` an organization member cannot hand their own preference to + * another member: `403 PERMISSION_DENIED`, the row keeps its owner and the + * other member lists nothing new; + * - `[org-admin]` / `[platform-admin]` an administrator cannot hand their own + * preference to another user either, and a member's preference is not + * theirs to reach by id (`404`, the object's own-row scope); + * - `[control]` an update that echoes the stored owner saves, and the owner + * reads, lists and updates their own preferences as before. + * + * Refusals assert `code` AND `status` (ADR-0112). + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { type VerifyStack } from '@objectstack/verify'; +import { bootShowcase } from './showcase-boot.js'; +import { showcaseAppDefaultSecurity } from './showcase-security.js'; + +const OBJ = 'sys_user_preference'; +const SYS = { isSystem: true } as const; + +const ALICE = 'pref.alice@verify.test'; +const BOB = 'pref.bob@verify.test'; +const CAROL = 'pref.carol.orgadmin@verify.test'; + +const recordOf = (b: any) => b?.record ?? b?.data ?? b; +const rowsOf = (b: any) => b?.records ?? b?.data ?? (Array.isArray(b) ? b : []); +const errorCodeOf = (b: any) => b?.error?.code ?? b?.code; + +describe('sys_user_preference over REST: a preference\'s owner is fixed when it is saved', () => { + let stack: VerifyStack; + let ql: any; + let aliceTok: string; + let bobTok: string; + let carolTok: string; + let adminTok: string; + let aliceId: string; + let bobId: string; + let carolId: string; + let adminId: string; + + const userIdOf = async (email: string): Promise => { + const id = (await ql.findOne('sys_user', { where: { email }, context: SYS }))?.id; + expect(id, `${email} provisioned`).toBeTruthy(); + return String(id); + }; + + /** Save a preference as `token`'s owner and return the created row. */ + const save = async (token: string, ownerId: string, key: string) => { + const r = await stack.apiAs(token, 'POST', `/data/${OBJ}`, { user_id: ownerId, key, value: ['first'] }); + expect(r.status, await r.clone().text()).toBe(201); + const row = recordOf(await r.json()); + expect(row?.id).toBeTruthy(); + return row; + }; + const storedOwner = async (id: string) => (await ql.findOne(OBJ, { where: { id }, context: SYS }))?.user_id; + + beforeAll(async () => { + stack = await bootShowcase({ security: showcaseAppDefaultSecurity() }); + adminTok = await stack.signIn(); + aliceTok = await stack.signUp(ALICE); + bobTok = await stack.signUp(BOB); + carolTok = await stack.signUp(CAROL); + ql = await stack.kernel.getServiceAsync('objectql'); + aliceId = await userIdOf(ALICE); + bobId = await userIdOf(BOB); + carolId = await userIdOf(CAROL); + adminId = await userIdOf('admin@objectos.ai'); + // CAROL holds the organization administrator's set, granted the way an + // administrator's grant is stored. + await ql.insert('sys_user_permission_set', { user_id: carolId, permission_set: 'organization_admin' }, { context: SYS }); + }, 180_000); + + afterAll(async () => { await stack?.stop?.(); }); + + it('[member] cannot hand their own preference to another member: 403, the row keeps its owner', async () => { + const row = await save(aliceTok, aliceId, 'ui.owner_fixed.member'); + const r = await stack.apiAs(aliceTok, 'PATCH', `/data/${OBJ}/${row.id}`, { user_id: bobId }); + expect(r.status, await r.clone().text()).toBe(403); + expect(errorCodeOf(await r.json())).toBe('PERMISSION_DENIED'); + expect(await storedOwner(row.id)).toBe(aliceId); + + const bobs = await stack.apiAs(bobTok, 'GET', `/data/${OBJ}`); + expect(bobs.status).toBe(200); + expect(rowsOf(await bobs.json()).map((p: any) => p.id)).not.toContain(row.id); + }); + + it('[org-admin] cannot hand their own preference to another user: 403, the row keeps its owner', async () => { + const row = await save(carolTok, carolId, 'ui.owner_fixed.org_admin'); + const r = await stack.apiAs(carolTok, 'PATCH', `/data/${OBJ}/${row.id}`, { user_id: bobId }); + expect(r.status, await r.clone().text()).toBe(403); + expect(errorCodeOf(await r.json())).toBe('PERMISSION_DENIED'); + expect(await storedOwner(row.id)).toBe(carolId); + }); + + it('[platform-admin] cannot hand their own preference to another user: 403, the row keeps its owner', async () => { + const row = await save(adminTok, adminId, 'ui.owner_fixed.platform_admin'); + const r = await stack.apiAs(adminTok, 'PATCH', `/data/${OBJ}/${row.id}`, { user_id: bobId }); + expect(r.status, await r.clone().text()).toBe(403); + expect(errorCodeOf(await r.json())).toBe('PERMISSION_DENIED'); + expect(await storedOwner(row.id)).toBe(adminId); + }); + + it.each([ + ['[org-admin]', () => carolTok], + ['[platform-admin]', () => adminTok], + ])('%s does not reach a member\'s preference by id: 404, the row keeps its owner', async (_who, tok) => { + const row = await save(aliceTok, aliceId, `ui.owner_fixed.reach.${_who.replace(/\W/g, '')}`); + const r = await stack.apiAs(tok(), 'PATCH', `/data/${OBJ}/${row.id}`, { user_id: bobId }); + expect(r.status, await r.clone().text()).toBe(404); + expect(errorCodeOf(await r.json())).toBe('RECORD_NOT_FOUND'); + expect(await storedOwner(row.id)).toBe(aliceId); + }); + + it('[control] an update that echoes the stored owner saves', async () => { + const row = await save(aliceTok, aliceId, 'ui.owner_fixed.echo'); + const r = await stack.apiAs(aliceTok, 'PATCH', `/data/${OBJ}/${row.id}`, { user_id: aliceId, value: ['second'] }); + expect(r.status, await r.clone().text()).toBe(200); + const stored = await ql.findOne(OBJ, { where: { id: row.id }, context: SYS }); + expect({ user_id: stored?.user_id, value: stored?.value }).toEqual({ user_id: aliceId, value: ['second'] }); + }); + + it('[control] the owner reads, lists and updates their own preference', async () => { + const row = await save(aliceTok, aliceId, 'ui.owner_fixed.own'); + const patch = await stack.apiAs(aliceTok, 'PATCH', `/data/${OBJ}/${row.id}`, { value: ['third'] }); + expect(patch.status, await patch.clone().text()).toBe(200); + const read = await stack.apiAs(aliceTok, 'GET', `/data/${OBJ}/${row.id}`); + expect(read.status).toBe(200); + expect(recordOf(await read.json())?.value).toEqual(['third']); + const list = await stack.apiAs(aliceTok, 'GET', `/data/${OBJ}`); + expect(rowsOf(await list.json()).map((p: any) => p.id)).toContain(row.id); + }); +}); From 424e92959b28d0338b269e7452de3c3fc994fad6 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 3 Nov 2026 00:40:10 +0000 Subject: [PATCH 3/3] docs(plugin-auth): the owner rule's header and changeset state the rule alone Claude-Session: https://claude.ai/code/session_01JQwxbWZsNmNDugrjfzMoJW Co-authored-by: Claude --- .changeset/29630-user-preference-owner-fixed.md | 2 +- packages/plugins/plugin-auth/src/owner-fixed-rule.ts | 12 ++++-------- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/.changeset/29630-user-preference-owner-fixed.md b/.changeset/29630-user-preference-owner-fixed.md index 4267e305564..4e058427538 100644 --- a/.changeset/29630-user-preference-owner-fixed.md +++ b/.changeset/29630-user-preference-owner-fixed.md @@ -14,6 +14,6 @@ Metadata-protocol: none - **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 moved a preference row to another user through `PATCH /api/v1/data/sys_user_preference/:id`. +- **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. diff --git a/packages/plugins/plugin-auth/src/owner-fixed-rule.ts b/packages/plugins/plugin-auth/src/owner-fixed-rule.ts index 3de88998f77..d8f930d1a53 100644 --- a/packages/plugins/plugin-auth/src/owner-fixed-rule.ts +++ b/packages/plugins/plugin-auth/src/owner-fixed-rule.ts @@ -2,21 +2,17 @@ /** * **The owner of a per-user row is fixed when the row is saved.** The one - * spelling of that rule, shared by every per-user object whose owner column is - * `user_id` rather than the engine-stamped `created_by`: `sys_personal_view` + * spelling of that rule, shared by `sys_personal_view` * (`personal-view-hooks.ts`) and `sys_user_preference` - * (`user-preference-hooks.ts`). + * (`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. * - * 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 for every object, beside the object's own row scope, and - * the post-image of a re-parent still carries the member's own `created_by`. - * A save rule is the one place that judges the CHANGE of owner. + * 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.