Skip to content

Commit bccd88e

Browse files
claudeclaude
committed
docs(adr): ADR-0092 — sys_user profile-field editing via guarded engine hook (#2784)
Evaluation outcome for RFC #2784: open a narrow, server-enforced profile write path on sys_user instead of keeping name/image edits endpoint-only or relying on UI readonly flags. - D1 field tiers: profile-editable (name, image) / admin-surface-only (role, phone_number, manager_id, ai_access, ban columns) / never-direct (email, credentials, system-managed stamps) - D2 fail-closed beforeUpdate whitelist guard in plugin-auth, user-context writes only (better-auth adapter + system writes bypass); also closes the existing admin_full_access raw-write hole on sys_user - D3 single whitelist module; import upsert's UPDATE_ALLOWED_FIELDS becomes a superset-by-construction re-export (anti-drift, PR #2771) - D4 userActions.edit flip ships only after the guard exists (ADR-0049) - D5 permission topology unchanged: platform-admin only; self-service stays on better-auth /update-user - D6 afterUpdate hook invalidates cached session snapshots Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YGAN5VvvBi4YRGwpNT6e21
1 parent 2a71f48 commit bccd88e

1 file changed

Lines changed: 281 additions & 0 deletions

File tree

‎docs/adr/0092-sys-user-profile-field-delegation.md‎

Lines changed: 281 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,281 @@
1+
# ADR-0092: sys_user profile-field editing — guarded edit affordance via engine hook
2+
3+
- **Status:** Proposed
4+
- **Date:** 2026-07-10
5+
- **Deciders:** ObjectStack Protocol Architects
6+
- **Relates to:** [ADR-0010](./0010-metadata-protection-model.md) (identity tables managed by better-auth), [ADR-0049](./0049-no-unenforced-security-properties.md) (no unenforced security properties), [ADR-0068](./0068-unified-user-context-and-built-in-identity-roles.md) (platform-admin gate), [ADR-0069](./0069-enterprise-authentication-hardening.md) (system-managed auth stamps), #2766 / PR #2771 (admin user management + identity import), #2784 (this RFC)
7+
8+
## TL;DR
9+
10+
`sys_user` is `managedBy: 'better-auth'`: every generic CRUD affordance is off and the
11+
default permission sets deny direct writes, because a raw ObjectQL write bypasses the
12+
better-auth pipeline (scrypt hashing, `sys_account` creation, verification flows,
13+
session-cache coherence). After #2766, every *credential* mutation has a purpose-built
14+
endpoint — but editing a **pure profile field** (a user's display name) still cannot go
15+
through the standard edit form / data API, which leaves the Users surface inconsistent
16+
with every other object.
17+
18+
Decision — open a **narrow, server-enforced** profile write path:
19+
20+
- **D1** — classify `sys_user` columns into three tiers: *profile-editable*
21+
(`name`, `image`), *admin-surface-only* (written only by dedicated endpoints /
22+
import: `role`, `phone_number`, `manager_id`, `ai_access`, ban columns), and
23+
*never-direct* (login identity, credentials, every system-managed stamp).
24+
- **D2** — enforcement lives in a **`beforeUpdate` engine hook** in `plugin-auth`
25+
that fail-closed strips any non-whitelisted field from a *user-context* update to
26+
`sys_user`. UI `readonly` flags alone are explicitly rejected as the mechanism
27+
(ADR-0049: a boundary the runtime does not enforce is worse than absent).
28+
- **D3** — one shared whitelist module in `plugin-auth`; the import upsert's
29+
`UPDATE_ALLOWED_FIELDS` (PR #2771) becomes a superset re-exported from it, so the
30+
two surfaces cannot drift.
31+
- **D4** — only after D2 lands, flip the UI affordance: `userActions: { edit: true }`
32+
on `sys_user` (create / import / delete stay off). Non-whitelisted fields must
33+
render non-editable in the standard edit form.
34+
- **D5** — permission sets are unchanged: members / org-admins keep
35+
`allowEdit: false`; the standard edit path is therefore **platform-admin only**.
36+
Self-service profile editing stays on better-auth `/update-user`
37+
(the existing `update_my_profile` action).
38+
- **D6** — an `afterUpdate` companion hook invalidates the affected user's cached
39+
session snapshots (secondary storage), keeping better-auth session reads coherent
40+
without delegating the write itself to `internalAdapter.updateUser`.
41+
42+
Side effect worth naming: D2 also **closes an existing hole** — today
43+
`admin_full_access` (wildcard, no RLS) can already write *any* `sys_user` column —
44+
including `email`, `banned`, `must_change_password` — through the generic data API;
45+
only UI affordances hide it. After D2 that path is whitelist-filtered too.
46+
47+
## Context
48+
49+
### Where writes stand after #2766
50+
51+
| Operation | Surface | Pipeline guarantees |
52+
|:---|:---|:---|
53+
| Create user | `/api/v1/auth/admin/create-user`, invite flow, `/admin/import-users` | scrypt hash, credential `sys_account`, `must_change_password` stamp |
54+
| Password | `/admin/set-user-password`, `/change-password`, reset flow | hashing, `password_changed_at`, session revocation |
55+
| Ban / unban / unlock / role | dedicated `/admin/*` endpoints | admin gate (ADR-0068), session invalidation |
56+
| Self profile (`name`, `image`) | better-auth `/update-user` via `update_my_profile` action | session-cache refresh |
57+
| **Admin edits another user's name** | **nothing** — no endpoint, no form | — |
58+
| Import upsert (existing users) | `/admin/import-users` | profile fields only, `UPDATE_ALLOWED_FIELDS = {name, phone_number, role}` |
59+
60+
The one missing cell is small but structurally annoying: fixing a typo in a teammate's
61+
display name requires either a CSV import round-trip or raw SQL. Meanwhile every other
62+
object in the platform offers inline/form editing gated by permissions.
63+
64+
### What the engine actually enforces today
65+
66+
Three findings from the current code shape this decision:
67+
68+
1. **Field-level `readonly` is a UI hint, not a server boundary.**
69+
`validateRecord` *skips* system/readonly columns rather than rejecting writes to
70+
them, and only `readonlyWhen` (conditional, state-dependent locks) is stripped
71+
server-side on update (`engine.ts` B2). Opening `edit` with only `readonly` flags
72+
would ship exactly the "parsed but unenforced security property" ADR-0049 bans.
73+
2. **The affordance matrix is advisory.** `resolveCrudAffordances` gates toolbar
74+
buttons; the REST data API for `sys_user` is fully on
75+
(`apiMethods: ['get','list','create','update','delete']`). The real gate is the
76+
permission-set layer — and `admin_full_access` passes it with a wildcard.
77+
3. **better-auth's own writes flow through the ObjectQL engine.** The better-auth
78+
adapter (`objectql-adapter.ts`) calls `engine.update(...)` with no caller context,
79+
which means (a) engine hooks *do* fire for better-auth writes — audit already
80+
captures them — and (b) a guard hook must distinguish user-context writes from
81+
internal/system writes, not just "writes".
82+
83+
### Session-cache consistency
84+
85+
ADR-0069 D2 wires the kernel cache service as better-auth `secondaryStorage`, which
86+
caches session (+ user snapshot) entries. better-auth's own update paths keep those
87+
snapshots coherent; a raw engine write to `sys_user` does not. For the D1 whitelist
88+
(`name`, `image`) staleness is cosmetic, but "cosmetic until someone widens the
89+
whitelist" is how drift ships — coherence is handled explicitly (D6).
90+
91+
## Decisions
92+
93+
### D1 — Field tiers
94+
95+
**Tier 1: profile-editable** (standard form / data API, guarded by D2):
96+
97+
- `name` — display name. No auth semantics (`displayNameField`, not a login key).
98+
- `image` — avatar URL. No auth semantics.
99+
100+
**Tier 2: admin-surface-only** — legitimate admin writes exist, but each has a
101+
dedicated surface with its own semantics; the generic form must not become a second
102+
door:
103+
104+
- `role`, `banned`, `ban_reason`, `ban_expires` — authorization state; dedicated
105+
endpoints revoke sessions / apply gates as side effects.
106+
- `phone_number` — a **login identifier** (unique index, sign-in key when the
107+
phoneNumber plugin is on). Import may upsert it (bulk identity onboarding is that
108+
surface's purpose); a form edit silently re-keying sign-in is not acceptable.
109+
If phone editing is later wanted, it needs a verification flow, not a text input.
110+
- `manager_id` — org-chart data, but it drives the `own_and_reports` hierarchy RLS
111+
scope (ADR-0057): writing it changes *who can read whose records*. Excluded from
112+
the profile tier; org-structure maintenance is its own surface (future issue if
113+
demand materialises).
114+
- `primary_business_unit_id` — denormalised projection maintained by plugin-sharing;
115+
never hand-edited (already documented on the field).
116+
- `ai_access` — a licensed-seat grant, capped by the enterprise AiSeatPlugin; must
117+
keep flowing through its enforcement path.
118+
119+
**Tier 3: never-direct** — no generic write under any actor:
120+
121+
- `email`, `email_verified` — login identity; changes require the better-auth
122+
change-email verification flow.
123+
- `two_factor_enabled` and all credential-adjacent state — owned by better-auth
124+
plugins.
125+
- All system-managed stamps: `password_changed_at`, `must_change_password`,
126+
`locked_until`, `failed_login_count`, `mfa_required_at`, `last_login_at`,
127+
`last_login_ip`, `source`, `id`, `created_at`, `updated_at`.
128+
129+
### D2 — A fail-closed `beforeUpdate` guard hook in plugin-auth
130+
131+
`plugin-auth` registers (at `kernel:ready`, same pattern as the existing SCIM
132+
provenance hook):
133+
134+
```
135+
engine.registerHook('beforeUpdate', guardSysUserProfileWrite,
136+
{ object: 'sys_user', packageId: 'com.objectstack.plugin-auth' });
137+
```
138+
139+
Behaviour:
140+
141+
- **Applies to user-context writes only**: when `hookContext.session` carries a real
142+
user and the operation context is not `isSystem`. Internal writes — the better-auth
143+
adapter (no context), plugin system writes (`SYSTEM_CTX`), import's engine calls —
144+
bypass the guard unchanged. This is what keeps sign-in stamps, ban endpoints and
145+
the import path working.
146+
- **Whitelist-filters, fail-closed**: every key in the update payload that is not in
147+
Tier 1 is stripped; if the payload becomes empty the hook throws a
148+
`FORBIDDEN`-class error (so a form that only tried to set `email` gets a loud
149+
failure, not a silent no-op). Unknown/new columns are non-whitelisted by
150+
construction — adding a field to `sys_user` never silently opens it.
151+
- **Covers both update shapes**: single-id updates and `options.multi` bulk updates
152+
run through the same `beforeUpdate` event; the filter applies to the payload in
153+
both cases.
154+
- Self-vs-other is *not* distinguished here: permission sets already decide who may
155+
update at all (D5); the hook decides *which columns* any permitted actor may touch.
156+
157+
Why a hook and not the alternatives:
158+
159+
- **Not UI `readonly` only** — finding #1 above; ADR-0049 prohibits it.
160+
- **Not full delegation to `internalAdapter.updateUser`** — the adapter itself writes
161+
through `engine.update`, so a hook that re-enters better-auth would recurse through
162+
the very pipeline it guards; it also couples the data path to better-auth API
163+
stability for zero gain, since the columns in Tier 1 have no auth-side effects.
164+
Delegation remains the right answer for anything credential-shaped — which is why
165+
those stay on dedicated endpoints (Tier 2/3), not in the whitelist.
166+
- **Not a new `/admin/update-profile` endpoint** — it would fix the one missing cell
167+
but keep `sys_user` off the standard UI path (the very inconsistency this RFC is
168+
about), add a bespoke audit surface, and leave the `admin_full_access` raw-write
169+
hole open.
170+
171+
Trade-off accepted: `admin_full_access`'s "rescue data directly" capability on
172+
`sys_user` narrows to Tier 1 via the HTTP data API. Rescue of other columns now
173+
requires system context (server-side script / CLI), which is deliberate hardening —
174+
the columns in question are exactly the ones raw rescue is most dangerous for.
175+
176+
### D3 — One whitelist module, import re-uses it
177+
178+
New module in `plugin-auth` (e.g. `sys-user-writable-fields.ts`):
179+
180+
```ts
181+
/** Tier 1 — standard form / data-API editable (guard hook, D2). */
182+
export const SYS_USER_PROFILE_EDIT_FIELDS = new Set(['name', 'image']);
183+
184+
/** Import-upsert may additionally touch these (admin bulk-identity surface). */
185+
export const SYS_USER_IMPORT_UPDATE_FIELDS = new Set([
186+
...SYS_USER_PROFILE_EDIT_FIELDS, 'phone_number', 'role',
187+
]);
188+
```
189+
190+
`admin-import-users.ts` replaces its private `UPDATE_ALLOWED_FIELDS` with
191+
`SYS_USER_IMPORT_UPDATE_FIELDS`. The relationship is subset-by-construction
192+
(a spread, not two hand-maintained lists), which is the actual anti-drift property
193+
the RFC asked for — the two surfaces intentionally differ (import may set `role` /
194+
`phone_number`; the form may not), so "the same set" would be wrong; "one file, one
195+
derivation" is right.
196+
197+
### D4 — Affordance flip, after enforcement exists
198+
199+
Once D2 is merged and tested, `sys_user` gains:
200+
201+
```ts
202+
userActions: { edit: true }, // create / import / delete stay bucket-default (off)
203+
```
204+
205+
`managedBy: 'better-auth'` stays — it remains true (drives permission-set defaults,
206+
system-field injection skip, docs) — the per-flag override is exactly what
207+
`userActions` exists for.
208+
209+
Form rendering: every non-Tier-1 field must render non-editable in the standard edit
210+
form. Most Tier 2/3 columns already carry `readonly: true`; the remainder (`email`,
211+
`email_verified`, `two_factor_enabled`, `role`, `banned`, `ban_reason`, `ban_expires`,
212+
`ai_access`) need field-level treatment at implementation time. Constraint for the
213+
implementer: `email` / `role` are referenced as action `params` (`create_user`,
214+
`invite_user`, `set_user_role`) — verify that flipping `readonly` on the field does
215+
not disable those param inputs before choosing between `readonly: true` and a
216+
form-level exclusion. Sequencing is a hard rule either way: **affordance ships in the
217+
same or a later release as the guard, never earlier** (ADR-0049).
218+
219+
### D5 — Who can edit whom: unchanged permission topology
220+
221+
- `member_default` / `viewer_readonly` / `organization_admin`: `allowEdit: false` on
222+
`sys_user` stays. Nothing about this ADR widens *who* may write.
223+
- Platform admins (`admin_full_access`) become the only principals whose standard-form
224+
edits reach the guard — and they were already past the permission layer today.
225+
- Self-service stays on better-auth `/update-user` (`update_my_profile` action):
226+
it already handles `name` / `image`, refreshes the session cache natively, and
227+
works for non-admin users, whom the permission layer (correctly) keeps away from
228+
the CRUD path. We do not build an RLS "self-row edit" carve-out for CRUD — one
229+
self-service door is enough, and better-auth's is strictly better.
230+
231+
If org-admin-scoped profile editing is wanted later ("org admin fixes a member's
232+
name"), that is a permission-set + RLS decision (`sys_user_org_members` is currently
233+
`select`-only) layered on top of the same guard — a follow-up, not this ADR.
234+
235+
### D6 — Session-cache invalidation companion hook
236+
237+
An `afterUpdate` hook (same registration site, `object: 'sys_user'`) invalidates the
238+
affected user's cached session entries in secondary storage when a user-context write
239+
changed a Tier-1 field. Implementation detail delegated to the auth manager (it owns
240+
the storage keys); the hook only reports "user X changed". No-op when secondary
241+
storage isn't wired (single-node memory cache TTLs it out).
242+
243+
### Audit (evaluation item 4 — no decision needed)
244+
245+
Nothing to build: plugin-audit already registers engine-wide `beforeUpdate` (previous
246+
snapshot) + `afterUpdate` (audit_log write) hooks, so guarded profile edits are
247+
captured with field-level before/after — *better* than the dedicated endpoints, whose
248+
audit trail is bespoke. This asymmetry is an argument for the hook mechanism, not
249+
against it.
250+
251+
## Alternatives considered
252+
253+
| Alternative | Verdict |
254+
|:---|:---|
255+
| Status quo + dedicated `/admin/update-profile` endpoint | Fixes the gap, keeps the UI inconsistency, adds bespoke audit, leaves the wildcard raw-write hole. Rejected. |
256+
| `userActions.edit: true` + field `readonly` flags only | UI-only boundary; server accepts any column from any permitted actor. Prohibited by ADR-0049. Rejected. |
257+
| Delegate profile writes to `internalAdapter.updateUser` inside the hook | Re-entrancy (adapter writes back through the engine), better-auth API coupling, no benefit for columns with no auth side effects. Rejected; D6 covers the one real coherence concern. |
258+
| Widen whitelist to `phone_number` / `manager_id` now | Login-identifier re-keying without verification; silent RLS-scope changes. Deferred behind dedicated flows. |
259+
260+
## Risks
261+
262+
- **Whitelist too narrow** → admins fall back to import/SQL for excluded fields.
263+
Acceptable: widening is a one-line, reviewed change against a named tier list.
264+
- **Guard bypass via system context** — any code path that stamps `isSystem` writes
265+
freely. That is today's status quo for *all* plugin writes and is required for
266+
better-auth itself; the guard's job is the *user-context* surface. Server-side
267+
code review remains the control for system-context writes.
268+
- **objectui behaviour** — the edit affordance flip surfaces a form whose field
269+
gating lives in the sibling repo. Implementation must verify the rendered form
270+
against a running stack (dogfood check) before the affordance ships.
271+
272+
## Rollout
273+
274+
1. Implementation issue (on acceptance): guard hook + shared whitelist module +
275+
import refactor + tests (user-context strip/throw, system-context bypass,
276+
multi-update, better-auth adapter path untouched).
277+
2. Session-cache invalidation hook (D6) — same PR or immediate follow-up.
278+
3. Affordance flip + field-level form gating + objectui verification — separate PR,
279+
gated on 1.
280+
4. Docs: update the identity/user-management guide to say profile fields are
281+
form-editable for platform admins; everything else keeps its dedicated action.

0 commit comments

Comments
 (0)