diff --git a/.changeset/30851-impersonation-identity-and-decider.md b/.changeset/30851-impersonation-identity-and-decider.md new file mode 100644 index 00000000000..64d1bc846ee --- /dev/null +++ b/.changeset/30851-impersonation-identity-and-decider.md @@ -0,0 +1,18 @@ +--- +'@objectstack/spec': patch +'@objectstack/plugin-auth': patch +'@objectstack/plugin-approvals': patch +'@objectstack/service-automation': patch +--- + +fix(audit): name the impersonating administrator on identity writes and on an approval decided while impersonating + +Clause-②: yes (widening) +Metadata-protocol: none + +Two writes an administrator makes while impersonating a user still read as the user's own in the audit ledger. Both now carry the same attribution-only `impersonatedBy` a record write from that session already carries, recorded as `sys_audit_log.metadata.impersonated_by`. `user_id`, `actor` and every permission stay the impersonated user's. + +- **Identity-table writes (`@objectstack/plugin-auth`).** A write better-auth makes on the user's behalf (renaming the profile, changing an email, a membership change, an admin route such as set-user-manager) runs as the system and is credited to the user through `attributedUserId`. On an impersonation session it now also names the administrator, read from the session row's `impersonatedBy`, never from the request body. `authSystemWriteContext()` builds it, and the adapter's `withSystemContext` writes now use that same builder. `resolveAttributedActor()` (new) answers the user and the administrator together; `runAttributedToUser()` also accepts the admitted actor object, whose new optional `impersonatedBy` the platform-admin gate fills from the session. +- **The approval decider's segment (`@objectstack/spec`, `@objectstack/plugin-approvals`, `@objectstack/service-automation`).** `ResumeSignal.impersonatedBy` (new, optional) names the administrator when the decider decided from an impersonation session. It is honoured only beside `resumedBy`, on a signal carrying `RESUME_AUTHORITY_SERVICE`, so an HTTP body cannot set it. The approvals decision door fills it from the decision request's context, and a stranded decision's journal keeps it for the replay. The nodes after the decision now carry exactly the decision's administrator, and none when the decider acted in their own name. Before this, a trigger user deciding their own run kept the stamp of the write that started it, and a decider deciding while impersonated carried none. + +No schema column and no migration. diff --git a/packages/plugins/plugin-approvals/src/approval-service.ts b/packages/plugins/plugin-approvals/src/approval-service.ts index b70ce85ff0f..3ae15095f2a 100644 --- a/packages/plugins/plugin-approvals/src/approval-service.ts +++ b/packages/plugins/plugin-approvals/src/approval-service.ts @@ -206,6 +206,14 @@ export interface ApprovalResumeSurface { * decided (the SLA sweep), and then the run keeps the identity it had. */ resumedBy?: string; + /** + * The administrator impersonating {@link resumedBy} when the decision came + * from an impersonation session (`ResumeSignal.impersonatedBy`), read off + * the decision request's context ({@link impersonatorOf}); the engine + * stamps it on the decider's segment. Absent when the decider acted in + * their own name, and never sent without `resumedBy`. + */ + impersonatedBy?: string; /** * #3801: the engine refuses a resume of an `approval` suspension unless * the signal carries this marker — the proof that the resume is the tail @@ -960,6 +968,13 @@ export interface StrandedContinuationSignal { * original carried none. */ resumedBy?: string; + /** + * The administrator the original resume named as impersonating + * {@link resumedBy} (`ResumeSignal.impersonatedBy`), so a replay stamps the + * decider's segment exactly as the decision did. Absent when the original + * carried none. + */ + impersonatedBy?: string; } /** @@ -1110,6 +1125,18 @@ function actingUserId(context: ExecutionContext | undefined): string | null { return typeof userId === 'string' && userId ? userId : null; } +/** + * The administrator a request's context names as impersonating its user — the + * session-resolved, attribution-only `ExecutionContext.impersonatedBy` — as a + * non-blank string, or `undefined`. Never read off a caller's input. + */ +function impersonatorOf(context: ExecutionContext | undefined): string | undefined { + const by = context?.impersonatedBy; + if (typeof by !== 'string') return undefined; + const trimmed = by.trim(); + return trimmed ? trimmed : undefined; +} + /** * The PERSON an action records (ADR-0118 D1): a `sys_user` id, or `null` — * never the slot the action took (that is `acted_as`), never an address the @@ -5268,7 +5295,7 @@ export class ApprovalService implements IApprovalService { */ private async serviceResume( runId: string, - signal: { output?: Record; branchLabel?: string; resumedBy?: string }, + signal: { output?: Record; branchLabel?: string; resumedBy?: string; impersonatedBy?: string }, ): Promise<{ runId: string; repairable: boolean; error: string } | undefined> { const result = await this.automation!.resume!(runId, { ...signal, [RESUME_AUTHORITY_SERVICE]: true }); const reported = result as @@ -5485,7 +5512,7 @@ export class ApprovalService implements IApprovalService { runId: string, requestId: string, what: string, - signal: { output?: Record; branchLabel?: string; resumedBy?: string }, + signal: { output?: Record; branchLabel?: string; resumedBy?: string; impersonatedBy?: string }, decision: string, ): Promise<{ resumed: boolean; resumeError?: string; resumeFailure?: ResumeFailureReport }> { const missing = this.missingRunCapability(runId, requestId, what, 'resume'); @@ -5546,6 +5573,7 @@ export class ApprovalService implements IApprovalService { await this.journalStrandedContinuation(requestId, { branchLabel: signal.branchLabel, output: signal.output, decision, what, ...(signal.resumedBy ? { resumedBy: signal.resumedBy } : {}), + ...(signal.resumedBy && signal.impersonatedBy ? { impersonatedBy: signal.impersonatedBy } : {}), }); } throw strandedDecisionFailure( @@ -5618,6 +5646,11 @@ export class ApprovalService implements IApprovalService { // vouches for nobody, so its auto-decision names no decider and the run // keeps the identity it had. const decider = actingUserId(context); + // The administrator at the keyboard when this decision came from an + // impersonation session, off the decision request's own context (the + // session), never off the input: the decider's segment names them, as + // the decision's own audit row does. Only beside a decider. + const impersonatedBy = decider ? impersonatorOf(context) : undefined; const outcome = await this.resumeRecordedOutcome( result.runId, requestId, `the ${result.decision} decision`, { @@ -5629,6 +5662,7 @@ export class ApprovalService implements IApprovalService { // whitelist already rejects them; this is defense in depth). output: { ...(result.outputs ?? {}), decision: result.decision, requestId }, ...(decider ? { resumedBy: decider } : {}), + ...(impersonatedBy ? { impersonatedBy } : {}), }, result.decision, ); @@ -8025,6 +8059,11 @@ export class ApprovalService implements IApprovalService { // The journalled decider (ruling A): the replayed nodes after the // decision run as the person who decided, not as the run's trigger. ...(typeof signal.resumedBy === 'string' && signal.resumedBy ? { resumedBy: signal.resumedBy } : {}), + // The journalled impersonator beside that decider: the replayed + // segment is stamped exactly as the decision's own resume would have. + ...(typeof signal.resumedBy === 'string' && signal.resumedBy + && typeof signal.impersonatedBy === 'string' && signal.impersonatedBy.trim() + ? { impersonatedBy: signal.impersonatedBy.trim() } : {}), }, signal.decision, ); diff --git a/packages/plugins/plugin-approvals/src/restored-approval-continuation.test.ts b/packages/plugins/plugin-approvals/src/restored-approval-continuation.test.ts index fd4690d5a9e..71deabd583e 100644 --- a/packages/plugins/plugin-approvals/src/restored-approval-continuation.test.ts +++ b/packages/plugins/plugin-approvals/src/restored-approval-continuation.test.ts @@ -948,7 +948,7 @@ describe('ruling A — a decision resumes the run as the person who decided', () let data: ReturnType; let service: ApprovalService; - let seen: Record; + let seen: Record; let rejectThrowsOnce: boolean; function boot() { @@ -963,7 +963,12 @@ describe('ruling A — a decision resumes the run as the person who decided', () throw new Error(DOWNSTREAM_FAILURE); } const ctx = resolveRunDataContext(context); - seen[node.id] = { userId: ctx?.userId, permissions: ctx?.permissions, ...(ctx?.isSystem ? { isSystem: true } : {}) }; + seen[node.id] = { + userId: ctx?.userId, permissions: ctx?.permissions, + ...(ctx?.isSystem ? { isSystem: true } : {}), + // Present only on a write the audit writer would stamp `impersonated_by`. + ...((ctx as any)?.impersonatedBy !== undefined ? { impersonatedBy: (ctx as any).impersonatedBy } : {}), + }; return { success: true }; }, } as never); @@ -1034,6 +1039,45 @@ describe('ruling A — a decision resumes the run as the person who decided', () expect(out.source).toBe('journal'); expect(seen.after_reject, 'the replay fell back to the trigger user').toEqual({ userId: 'u1', permissions: ['deal_manager'] }); }); + + // #30851: an administrator impersonating the decider made THIS decision, so + // the decider's segment names them; read off the decision request's context + // (the session), never off the input. + const IMPERSONATED_DECIDER_CTX = { ...DECIDER_CTX, impersonatedBy: 'adm' } as any; + + it.each(['approve', 'reject'] as const)('%s while impersonated: the nodes after the decision carry the administrator beside the decider', async (decision) => { + const automation = boot(); + const req = await parked(automation); + const out = await service.decide(req.id, { decision, actorId: 'u1', comment: decision }, IMPERSONATED_DECIDER_CTX); + expect(out.resumed, JSON.stringify(out)).toBe(true); + expect(seen[`after_${decision}`]).toEqual({ userId: 'u1', permissions: ['deal_manager'], impersonatedBy: 'adm' }); + }); + + it('CONTROL: an administrator named in the decision INPUT is not the session, and stamps nothing', async () => { + const automation = boot(); + const req = await parked(automation); + const out = await service.decide( + req.id, { decision: 'approve', actorId: 'u1', comment: 'ok', impersonatedBy: 'forged' } as any, DECIDER_CTX, + ); + expect(out.resumed, JSON.stringify(out)).toBe(true); + expect(seen.after_approve).toEqual({ userId: 'u1', permissions: ['deal_manager'] }); + }); + + it('a stranded impersonated decision is replayed with the same administrator beside the decider', async () => { + const automation = boot(); + const req = await parked(automation); + rejectThrowsOnce = true; + const err = await service.decide(req.id, { decision: 'reject', actorId: 'u1', comment: 'no' }, IMPERSONATED_DECIDER_CTX) + .then(() => null, (e: Error) => e); + expect(err?.message).toMatch(/^RESUME_FAILED/); + const journal = JSON.parse((await rowOf(req.id)).node_config_json).__strandedContinuation; + expect(journal).toMatchObject({ decision: 'reject', resumedBy: 'u1', impersonatedBy: 'adm' }); + + expect((await automation.restoreConsumedSuspension(req.flow_run_id)).restored).toBe(true); + const out = await service.continueRestoredRun(req.id, { requestedBy: 'ops', reason: 'fixed the record' }); + expect(out.resumed, JSON.stringify(out)).toBe(true); + expect(seen.after_reject).toEqual({ userId: 'u1', permissions: ['deal_manager'], impersonatedBy: 'adm' }); + }); }); /** diff --git a/packages/plugins/plugin-auth/src/admin-impersonate-endpoint.test.ts b/packages/plugins/plugin-auth/src/admin-impersonate-endpoint.test.ts index effacb01c22..cec3b705d8f 100644 --- a/packages/plugins/plugin-auth/src/admin-impersonate-endpoint.test.ts +++ b/packages/plugins/plugin-auth/src/admin-impersonate-endpoint.test.ts @@ -284,6 +284,58 @@ describe.concurrent('ADR-0068 D2 — /admin/impersonate-user admits the ObjectSt }); }); +// ─────────────────────────────────────────────────────────────────────────── +// #30851 — an identity write made from the impersonation session names the +// administrator beside the impersonated user it is credited to. The engine's +// `buildProvenance` copies `impersonatedBy` onto the hook context, where the +// audit writer records it as `sys_audit_log.metadata.impersonated_by`. +describe.concurrent('an identity write made while impersonating names the administrator', () => { + /** Every `sys_user` update's context, as the better-auth adapter hands it to the engine. */ + const recordUserWrites = (engine: any) => { + const writes: Array<{ data: any; context: any }> = []; + const update = engine.update.bind(engine); + engine.update = async (name: string, data: any, options?: any) => { + if (name === 'sys_user') writes.push({ data, context: options?.context }); + return update(name, data, options); + }; + return writes; + }; + const updateUser = (manager: AuthManager, bearer: string, body: Record) => + manager.handleRequest( + new Request(`${BASE}/update-user`, { + method: 'POST', + headers: { 'Content-Type': 'application/json', authorization: `Bearer ${bearer}` }, + body: JSON.stringify(body), + }), + ); + + it('a rename from the impersonation session carries the administrator, read off the session and not the body', async () => { + const { engine, manager, adminBearer, adminId, targetId } = await arrange(); + const writes = recordUserWrites(engine); + const started = await impersonate(manager, adminBearer, targetId); + expect(started.status).toBe(200); + + const renamed = await updateUser(manager, bearerFrom(started), { name: 'Renamed by admin', impersonatedBy: 'usr_forged' }); + expect(renamed.status).toBe(200); + const write = writes.find((w) => w.data?.name === 'Renamed by admin'); + expect(write, JSON.stringify(writes)).toBeDefined(); + expect(write!.context).toMatchObject({ isSystem: true, attributedUserId: targetId, impersonatedBy: adminId }); + expect(write!.context.userId, 'the write still authorizes as the system').toBeUndefined(); + }); + + it("CONTROL: the administrator's own rename names no administrator", async () => { + const { engine, manager, adminBearer, adminId } = await arrange(); + const writes = recordUserWrites(engine); + + const renamed = await updateUser(manager, adminBearer, { name: 'Renamed self' }); + expect(renamed.status).toBe(200); + const write = writes.find((w) => w.data?.name === 'Renamed self'); + expect(write, JSON.stringify(writes)).toBeDefined(); + expect(write!.context).toMatchObject({ isSystem: true, attributedUserId: adminId }); + expect('impersonatedBy' in write!.context).toBe(false); + }); +}); + // ─────────────────────────────────────────────────────────────────────────── describe.concurrent('the other direction — a non-entitled caller is still refused', () => { it('a signed-in plain member is refused 403 YOU_ARE_NOT_ALLOWED_TO_IMPERSONATE_USERS', async () => { diff --git a/packages/plugins/plugin-auth/src/auth-actor-attribution.test.ts b/packages/plugins/plugin-auth/src/auth-actor-attribution.test.ts index c7e2362ec1c..2c957289119 100644 --- a/packages/plugins/plugin-auth/src/auth-actor-attribution.test.ts +++ b/packages/plugins/plugin-auth/src/auth-actor-attribution.test.ts @@ -16,8 +16,10 @@ import { describe, it, expect, vi } from 'vitest'; import type { IDataEngine } from '@objectstack/core'; import { runWithAuthActorScope, + runAttributedToUser, setAuthActorResolver, resolveAttributedUserId, + resolveAttributedActor, authSystemWriteContext, } from './auth-actor-attribution'; import { withSystemContext } from './objectql-adapter'; @@ -204,3 +206,68 @@ describe('withSystemContext carries attribution on WRITES only (#4586)', () => { }); }); }); + +// The real-pipeline pin (an impersonated `/update-user` on better-auth) lives +// with the impersonation harness, in `admin-impersonate-endpoint.test.ts`. +describe('an impersonation session names its administrator on identity writes (#30851)', () => { + it('the context carries the administrator beside the impersonated user, attribution only', async () => { + const ctx = await runWithAuthActorScope(async () => { + setAuthActorResolver(async () => ({ userId: 'usr_target', impersonatedBy: 'usr_admin' })); + return authSystemWriteContext(); + }); + expect(ctx).toEqual({ isSystem: true, attributedUserId: 'usr_target', impersonatedBy: 'usr_admin' }); + expect(ctx).not.toHaveProperty('userId'); + }); + + it('CONTROL: a session that is not an impersonation carries no administrator', async () => { + for (const answer of ['usr_target', { userId: 'usr_target' }, { userId: 'usr_target', impersonatedBy: null }, { userId: 'usr_target', impersonatedBy: ' ' }]) { + const ctx = await runWithAuthActorScope(async () => { + setAuthActorResolver(async () => answer); + return authSystemWriteContext(); + }); + expect(ctx, JSON.stringify(answer)).toEqual({ isSystem: true, attributedUserId: 'usr_target' }); + } + }); + + it('CONTROL: an administrator with no user beside it attributes nothing', async () => { + const actor = await runWithAuthActorScope(async () => { + setAuthActorResolver(async () => ({ userId: null, impersonatedBy: 'usr_admin' })); + return resolveAttributedActor(); + }); + expect(actor).toBeUndefined(); + }); + + it('withSystemContext stamps it on insert / update / delete, never on reads', async () => { + const engine = { + insert: vi.fn().mockResolvedValue({ id: '1' }), + update: vi.fn().mockResolvedValue({ id: '1' }), + delete: vi.fn().mockResolvedValue(undefined), + find: vi.fn().mockResolvedValue([]), + findOne: vi.fn().mockResolvedValue({ id: '1' }), + count: vi.fn().mockResolvedValue(0), + } as unknown as IDataEngine; + await runWithAuthActorScope(async () => { + setAuthActorResolver(async () => ({ userId: 'usr_target', impersonatedBy: 'usr_admin' })); + const e = withSystemContext(engine); + await e.insert('sys_user', { id: 'u1' } as any); + await e.update('sys_user', { id: 'u1' } as any); + await e.delete('sys_user', { where: { id: 'u1' } } as any); + await e.find('sys_user', { where: {} }); + }); + const expected = { context: { attributedUserId: 'usr_target', impersonatedBy: 'usr_admin', isSystem: true } }; + expect(engine.insert).toHaveBeenCalledWith('sys_user', { id: 'u1' }, expected); + expect(engine.update).toHaveBeenCalledWith('sys_user', { id: 'u1' }, expected); + expect(engine.delete).toHaveBeenCalledWith('sys_user', expect.objectContaining(expected)); + expect(engine.find).toHaveBeenCalledWith('sys_user', expect.objectContaining({ context: { isSystem: true } })); + }); + + it('a framework route that gated the admitted actor carries the administrator it names', async () => { + const impersonated = await runAttributedToUser({ id: 'usr_target', impersonatedBy: 'usr_admin' }, () => authSystemWriteContext()); + expect(impersonated).toEqual({ isSystem: true, attributedUserId: 'usr_target', impersonatedBy: 'usr_admin' }); + const plain = await runAttributedToUser({ id: 'usr_admin' }, () => authSystemWriteContext()); + expect(plain).toEqual({ isSystem: true, attributedUserId: 'usr_admin' }); + const bare = await runAttributedToUser('usr_admin', () => authSystemWriteContext()); + expect(bare).toEqual({ isSystem: true, attributedUserId: 'usr_admin' }); + }); +}); + diff --git a/packages/plugins/plugin-auth/src/auth-actor-attribution.ts b/packages/plugins/plugin-auth/src/auth-actor-attribution.ts index 63839875b93..e7f7674ce16 100644 --- a/packages/plugins/plugin-auth/src/auth-actor-attribution.ts +++ b/packages/plugins/plugin-auth/src/auth-actor-attribution.ts @@ -63,17 +63,36 @@ import { AsyncLocalStorage } from 'node:async_hooks'; import { agentForwardPerformedBy } from './agent-forward-identity-view.js'; /** - * Resolves the acting user id for the current auth request, or `null` when no + * The human a better-auth write is credited to, as the session names them. + * + * `impersonatedBy` is the administrator at the keyboard when that session is + * an impersonation session (better-auth's admin plugin stamps it on the + * session row): the write is still credited to `userId`, the impersonated + * user, and `impersonatedBy` rides beside it as the attribution-only + * `ExecutionContext.impersonatedBy` the audit writer records as + * `impersonated_by`, the key a record write made from the same session + * already carries. Read off the SESSION, never off a request body. + */ +export interface AttributedAuthActor { + userId: string; + impersonatedBy?: string; +} + +/** + * Resolves the acting user for the current auth request, or `null` when no * session can be established. Supplied by the hook layer, which is the only - * place the session actor exists. + * place the session actor exists. A bare string is the user id alone; the + * object form also names the impersonating administrator, when there is one. */ -export type AuthActorResolver = () => Promise; +export type AuthActorResolver = () => Promise< + string | { userId?: string | null; impersonatedBy?: string | null } | null | undefined +>; interface AuthActorScope { /** Set by the before-hook; run at most once, on first demand. */ resolver?: AuthActorResolver; /** Memoized resolution for this request (shared by concurrent writes). */ - pending?: Promise; + pending?: Promise; /** * True only INSIDE the resolver's own async subtree. Session resolution goes * back through better-auth and can itself write (session refresh); that write @@ -106,11 +125,12 @@ export function runWithAuthActorScope(fn: () => Promise): Promise { * the system, and this route's own authorization already happened, above. */ export function runAttributedToUser( - userId: string | undefined, + actor: string | { id: string; impersonatedBy?: string | null } | undefined, fn: () => Promise, ): Promise { return runWithAuthActorScope(async () => { - if (userId) setAuthActorResolver(async () => userId); + const known = typeof actor === 'string' ? { userId: actor } : actor ? { userId: actor.id, impersonatedBy: actor.impersonatedBy } : undefined; + if (known?.userId) setAuthActorResolver(async () => known); return fn(); }); } @@ -139,6 +159,19 @@ export function setAuthActorResolver(resolver: AuthActorResolver): void { * must never fail the write it was describing. */ export async function resolveAttributedUserId(): Promise { + return (await resolveAttributedActor())?.userId; +} + +/** + * The acting user for the current auth request together with the + * administrator impersonating them, or `undefined` when there is no acting + * user (the same cases as {@link resolveAttributedUserId}). `impersonatedBy` + * is present only for an impersonation session, and only as a non-blank + * string; without a user it is never answered on its own. + * + * Never throws, for the reason {@link resolveAttributedUserId} gives. + */ +export async function resolveAttributedActor(): Promise { const scope = scopeStore.getStore(); if (!scope || scope.suspended) return undefined; if (!scope.pending) { @@ -146,8 +179,7 @@ export async function resolveAttributedUserId(): Promise { if (!resolver) return undefined; scope.pending = scopeStore.run({ suspended: true }, async () => { try { - const resolved = await resolver(); - return typeof resolved === 'string' && resolved.length > 0 ? resolved : undefined; + return attributedActorOf(await resolver()); } catch { return undefined; } @@ -156,6 +188,19 @@ export async function resolveAttributedUserId(): Promise { return scope.pending; } +/** A resolver's answer as an {@link AttributedAuthActor}, or `undefined` when it names no user. */ +function attributedActorOf(resolved: Awaited>): AttributedAuthActor | undefined { + const nonBlank = (v: unknown): string | undefined => + typeof v === 'string' && v.trim().length > 0 ? v.trim() : undefined; + if (typeof resolved === 'string') { + return resolved.length > 0 ? { userId: resolved } : undefined; + } + const userId = typeof resolved?.userId === 'string' && resolved.userId.length > 0 ? resolved.userId : undefined; + if (!userId) return undefined; + const impersonatedBy = nonBlank(resolved?.impersonatedBy); + return impersonatedBy ? { userId, impersonatedBy } : { userId }; +} + /** * The execution context for a better-auth-owned write: system authorization * plus — when one is in scope — the human it is attributed to. @@ -168,9 +213,17 @@ export async function resolveAttributedUserId(): Promise { export async function authSystemWriteContext(): Promise<{ isSystem: true; attributedUserId?: string; + impersonatedBy?: string; performedBy?: { clientId: string }; }> { - const attributedUserId = await resolveAttributedUserId(); + const actor = await resolveAttributedActor(); + const attributedUserId = actor?.userId; + // An impersonation session's write is credited to the impersonated user + // (above) and names the administrator at the keyboard beside it, as the + // attribution-only `ExecutionContext.impersonatedBy` a record write from the + // same session carries: the audit writer records it as `impersonated_by`. + // Read off the session by the resolver, never off the request body. + const impersonatedBy = actor?.impersonatedBy; // [ADR-0090 D10 rule 5, #28367] A forwarded AI agent's identity write is // attributed to both: the user (above) and the agent's client, which the // audit row records as `performed_by`. Attribution only, like the user half. @@ -178,6 +231,7 @@ export async function authSystemWriteContext(): Promise<{ return { isSystem: true, ...(attributedUserId ? { attributedUserId } : {}), + ...(impersonatedBy ? { impersonatedBy } : {}), ...(performedBy ? { performedBy } : {}), }; } diff --git a/packages/plugins/plugin-auth/src/auth-manager-actor-guards.ts b/packages/plugins/plugin-auth/src/auth-manager-actor-guards.ts index a2dcd90cca8..2418f73360d 100644 --- a/packages/plugins/plugin-auth/src/auth-manager-actor-guards.ts +++ b/packages/plugins/plugin-auth/src/auth-manager-actor-guards.ts @@ -126,7 +126,15 @@ export interface ActorGuardHost { export async function resolveActor( ctx: any, options?: { failedReadThrows?: boolean }, -): Promise<{ userId: string; activeOrgId?: string } | null> { +): Promise<{ userId: string; activeOrgId?: string; impersonatedBy?: string } | null> { + // The administrator an impersonation session names (better-auth's admin + // plugin stamps `impersonatedBy` on the session row), read off the SESSION + // only: the attribution seam carries it beside the user as the + // attribution-only `impersonatedBy` of an identity write. + const impersonatorOn = (session: any): { impersonatedBy?: string } => { + const by = session?.impersonatedBy ?? session?.impersonated_by; + return typeof by === 'string' && by.trim().length > 0 ? { impersonatedBy: by.trim() } : {}; + }; try { const { getSessionFromCtx } = await import('better-auth/api'); const s: any = await getSessionFromCtx(ctx as any); @@ -136,6 +144,7 @@ export async function resolveActor( userId: String(userId), activeOrgId: s?.session?.activeOrganizationId ?? s?.activeOrganizationId ?? undefined, + ...impersonatorOn(s?.session), }; } } catch { /* fall through to explicit token resolution */ } @@ -181,6 +190,7 @@ export async function resolveActor( userId, activeOrgId: sess.activeOrganizationId ?? sess.active_organization_id ?? undefined, + ...impersonatorOn(sess), }; } } diff --git a/packages/plugins/plugin-auth/src/auth-manager-before-hook.ts b/packages/plugins/plugin-auth/src/auth-manager-before-hook.ts index 09d7b2f1f49..923fa31b763 100644 --- a/packages/plugins/plugin-auth/src/auth-manager-before-hook.ts +++ b/packages/plugins/plugin-auth/src/auth-manager-before-hook.ts @@ -130,9 +130,13 @@ export async function globalBeforeHook( // `withSystemContext`): better-auth already authorized them under // its own ACL, and re-authorizing as the human would open the // second adjudication track ADR-0095 D3 closed. + // + // An impersonation session also names the administrator at the + // keyboard: it travels beside the user as the attribution-only + // `impersonatedBy`, read off that same session, never off the body. setAuthActorResolver(async () => { const actor = await this.resolveActor(ctx); - return actor?.userId ?? null; + return actor ? { userId: actor.userId, impersonatedBy: actor.impersonatedBy } : null; }); // ── #23813: a minted principal session is never refreshed ─────── diff --git a/packages/plugins/plugin-auth/src/auth-manager.ts b/packages/plugins/plugin-auth/src/auth-manager.ts index f49e0c4a690..20d509e0696 100644 --- a/packages/plugins/plugin-auth/src/auth-manager.ts +++ b/packages/plugins/plugin-auth/src/auth-manager.ts @@ -4936,7 +4936,7 @@ export class AuthManager { private resolveActor( ctx: any, options?: { failedReadThrows?: boolean }, - ): Promise<{ userId: string; activeOrgId?: string } | null> { + ): Promise<{ userId: string; activeOrgId?: string; impersonatedBy?: string } | null> { // [#25224 cut 1] The body moved to `auth-manager-actor-guards.ts`; it reads no member of this class. return resolveActor(ctx, options); } diff --git a/packages/plugins/plugin-auth/src/auth-plugin.ts b/packages/plugins/plugin-auth/src/auth-plugin.ts index b4619a86de3..c438d171ff4 100644 --- a/packages/plugins/plugin-auth/src/auth-plugin.ts +++ b/packages/plugins/plugin-auth/src/auth-plugin.ts @@ -3605,7 +3605,14 @@ export class AuthPlugin implements Plugin { const session: SessionRead | null = await authApi.getSession(inProcessSessionReadInput(c.req.raw.headers)); const verdict = judgePlatformAdmin(session); if (!verdict.ok) return c.json(verdict.refusal.body, verdict.refusal.status); - return verdict.actor; + // An impersonation session names the administrator at the keyboard; the + // route's identity writes carry it beside the admitted actor as the + // attribution-only `impersonatedBy` (`runAttributedToUser`), read off + // this session and never off the body. Authorization is unchanged. + const impersonatedBy = session?.session?.impersonatedBy; + return typeof impersonatedBy === 'string' && impersonatedBy.trim() !== '' + ? { ...verdict.actor, impersonatedBy: impersonatedBy.trim() } + : verdict.actor; }; // ── [#24877] ADR-0069 D5/D7 — better-auth's universal handler for the @@ -3733,7 +3740,7 @@ export class AuthPlugin implements Plugin { // Attribution only — the route's own authorization already happened // in `gateAdmin`. Opening the actor seam here credits the `sys_user` // row to the admin instead of recording it as the system. - const { status, body } = await runAttributedToUser(actor.id, () => + const { status, body } = await runAttributedToUser(actor, () => runSetUserManager( { getDataEngine: () => this.authManager!.getDataEngine() as any, @@ -3904,7 +3911,7 @@ export class AuthPlugin implements Plugin { // binds in `user.create.after` — are credited to the admin instead // of recorded as the system. Attribution only: the route's own // authorization already happened above, and the writes stay system. - const { status, body } = await runAttributedToUser(actor.id, () => + const { status, body } = await runAttributedToUser(actor, () => runAdminCreateUser( { ...adminUserDeps(), @@ -3979,7 +3986,7 @@ export class AuthPlugin implements Plugin { // write is driven server-side and never passes through // `AuthManager.handleRequest`, so the request-scoped actor seam is // not open unless we open it here. - const { status, body } = await runAttributedToUser(actor.id, () => + const { status, body } = await runAttributedToUser(actor, () => runAdminBanUser(adminBanDeps(), actor, c.req.raw), ); return c.json(body, status as any); @@ -3996,7 +4003,7 @@ export class AuthPlugin implements Plugin { if (actor instanceof Response) return actor; const unavailable = await adminPluginMissing(c); if (unavailable) return unavailable; - const { status, body } = await runAttributedToUser(actor.id, () => + const { status, body } = await runAttributedToUser(actor, () => runAdminUnbanUser(adminBanDeps(), actor, c.req.raw), ); return c.json(body, status as any); @@ -4194,7 +4201,7 @@ export class AuthPlugin implements Plugin { })(); // [#4586] Same seam as create-user: server-side better-auth calls, // credited to the admin who ran the import. - const { status, body } = await runAttributedToUser(actor.id, () => + const { status, body } = await runAttributedToUser(actor, () => runAdminImportUsers( { getAuthApi: () => this.authManager!.getApi() as any, @@ -4248,7 +4255,7 @@ export class AuthPlugin implements Plugin { // and never passes through `AuthManager.handleRequest`, so open the // attribution seam here — the `sys_member` row this creates is // credited to the admin instead of recorded as the system. - const { status, body } = await runAttributedToUser(actor.id, () => + const { status, body } = await runAttributedToUser(actor, () => runOrganizationAddMember({ getAuthApi: () => this.authManager!.getApi() as any }, c.req.raw), ); return c.json(body, status as any); diff --git a/packages/plugins/plugin-auth/src/objectql-adapter.ts b/packages/plugins/plugin-auth/src/objectql-adapter.ts index e090fb74ed7..1330e4f33c7 100644 --- a/packages/plugins/plugin-auth/src/objectql-adapter.ts +++ b/packages/plugins/plugin-auth/src/objectql-adapter.ts @@ -4,8 +4,7 @@ import type { IDataEngine } from '@objectstack/core'; import { createAdapterFactory } from 'better-auth/adapters'; import type { CleanedWhere, WhereOperator } from 'better-auth/adapters'; import { SystemObjectName } from '@objectstack/spec/system'; -import { resolveAttributedUserId } from './auth-actor-attribution.js'; -import { agentForwardPerformedBy } from './agent-forward-identity-view.js'; +import { authSystemWriteContext } from './auth-actor-attribution.js'; import { inScimRequestScope } from './scim-connection-service.js'; import { adoptExistingMembership } from './adopt-membership.js'; import { refuseIssuerRepointWithLiveBindings } from './account-identity-preflight.js'; @@ -751,22 +750,14 @@ export function withSystemContext(engine: IDataEngine): IDataEngine { const asSystem = (q: any) => ({ ...(q ?? {}), context: { isSystem: true, ...(q?.context ?? {}) } }); // The attributed human is spread FIRST so an explicit caller-supplied context // still wins on every key — the same precedence `isSystem` already had. - const asAttributedSystem = async (q: any) => { - const attributedUserId = await resolveAttributedUserId(); - // [ADR-0090 D10 rule 5, #28367] A forwarded AI agent's write also names the - // agent's client (`performedBy`), so the audit row records both sides. - const performedBy = agentForwardPerformedBy(); - if (!attributedUserId && !performedBy) return asSystem(q); - return { - ...(q ?? {}), - context: { - ...(attributedUserId ? { attributedUserId } : {}), - ...(performedBy ? { performedBy } : {}), - isSystem: true, - ...(q?.context ?? {}), - }, - }; - }; + // The context is the one `authSystemWriteContext()` builds for every + // better-auth-owned write: the attributed human, the administrator + // impersonating them on an impersonation session, and a forwarded AI agent's + // client ([ADR-0090 D10 rule 5, #28367]), so the audit row records each side. + const asAttributedSystem = async (q: any) => ({ + ...(q ?? {}), + context: { ...(await authSystemWriteContext()), ...(q?.context ?? {}) }, + }); return { insert: async (m: string, d: any, o?: any) => e.insert(m, d, await asAttributedSystem(o)), update: async (m: string, d: any, o?: any) => e.update(m, d, await asAttributedSystem(o)), diff --git a/packages/plugins/plugin-auth/src/platform-admin-gate.ts b/packages/plugins/plugin-auth/src/platform-admin-gate.ts index a7015460cf1..8cecd3d9057 100644 --- a/packages/plugins/plugin-auth/src/platform-admin-gate.ts +++ b/packages/plugins/plugin-auth/src/platform-admin-gate.ts @@ -68,6 +68,13 @@ type SessionRead = Awaited> export interface PlatformAdminActor { id: string; email?: string; + /** + * The administrator impersonating {@link id} when the gated session is an + * impersonation session — attribution only, set by the route's gate from + * that session (never by {@link judgePlatformAdmin}, whose verdict it does + * not enter), and carried onto the route's identity writes. + */ + impersonatedBy?: string; } /** ADR-0112 refusal envelope — the ObjectStack shape, not better-auth's. */ diff --git a/packages/services/service-automation/src/engine.ts b/packages/services/service-automation/src/engine.ts index 12b95f6db2f..0d024d71bbc 100644 --- a/packages/services/service-automation/src/engine.ts +++ b/packages/services/service-automation/src/engine.ts @@ -6069,7 +6069,17 @@ export class AutomationEngine implements IAutomationService { * - a decider the signal names WITHOUT the service marker: only the * service that authenticated and recorded the decision may name its * decider, and an HTTP body cannot carry the symbol; - * - a decider who is already the run's acting user. + * - a decider who is already the run's acting user, but for the + * impersonation stamp below. + * + * Whoever decides, the decider's segment carries exactly the decision's + * own {@link ResumeSignal.impersonatedBy} as its + * {@link AutomationContext.impersonatedBy} — the administrator who decided + * while impersonating the decider, as the owning service read it off the + * decision request's session — and none when the decider acted in their + * own name. The stamp of the write that started the run never survives + * into a human decider's segment. Attribution only, like the stamp it + * replaces. * * On a resolution failure the decider's segment carries no positions or * permission sets, so its data operations fall back to the baseline member diff --git a/packages/services/service-automation/src/resume-decider-identity.integration.test.ts b/packages/services/service-automation/src/resume-decider-identity.integration.test.ts index 5d3cc6bc9e2..f8d60530e57 100644 --- a/packages/services/service-automation/src/resume-decider-identity.integration.test.ts +++ b/packages/services/service-automation/src/resume-decider-identity.integration.test.ts @@ -416,6 +416,93 @@ describe('a run started by an impersonation session\'s write names the administr }); }); +/** A human decision made from an impersonation session, as `ApprovalService.decide` hands it over. */ +const decidedImpersonated = (branchLabel: 'approve' | 'reject', resumedBy: string, impersonatedBy: string): ResumeSignal => ({ + ...decided(branchLabel, resumedBy), + impersonatedBy, +}); + +describe("the decider's segment names exactly the DECISION's impersonating administrator, never the trigger's", () => { + for (const runAs of ['user', 'system'] as const) { + it(`runAs:'${runAs}': (a) the trigger user deciding in their own name drops the trigger-time stamp`, async () => { + const h = envelopeHarness(); + h.engine.registerFlow('review', reviewFlow('review', { recorder: true, runAs }) as never); + const paused = await h.engine.execute('review', { ...TRIGGER, impersonatedBy: 'usr_admin' } as never); + expect(h.ctxOf('submit').impersonatedBy, 'premise: the trigger carried the administrator').toBe('usr_admin'); + + await h.engine.resume(paused.runId!, decided('approve', 'usr_rep')); + const after = h.ctxOf('after1'); + expect(after.userId, 'the decider is the run user, so the identity is unchanged').toBe('usr_rep'); + expect('impersonatedBy' in after, 'the decider acted in their own name').toBe(false); + }); + + it(`runAs:'${runAs}': (b) a decider deciding while impersonated stamps the decision's administrator`, async () => { + const h = envelopeHarness(); + h.engine.registerFlow('review', reviewFlow('review', { recorder: true, runAs }) as never); + const paused = await h.engine.execute('review', { ...TRIGGER, impersonatedBy: 'usr_admin' } as never); + + await h.engine.resume(paused.runId!, decidedImpersonated('approve', 'usr_mgr', 'usr_admin2')); + const after = h.ctxOf('after1'); + expect(after.userId, 'the subject stays the decider').toBe('usr_mgr'); + expect(after.impersonatedBy, "the decision's administrator, not the trigger's").toBe('usr_admin2'); + }); + } + + it('(b) the trigger user deciding while impersonated takes the stamp on an unstamped run', async () => { + const h = envelopeHarness(); + h.engine.registerFlow('review', reviewFlow('review', { recorder: true }) as never); + const paused = await h.engine.execute('review', { ...TRIGGER } as never); + + await h.engine.resume(paused.runId!, decidedImpersonated('approve', 'usr_rep', 'usr_admin')); + const after = h.ctxOf('after1'); + expect(after.userId).toBe('usr_rep'); + expect(after.impersonatedBy).toBe('usr_admin'); + }); + + it("(c) a plain decider on a plain run carries none, and a later plain decision drops the first decision's stamp", async () => { + const h = envelopeHarness(); + h.engine.registerFlow('review', reviewFlow('review', { recorder: true }) as never); + const paused = await h.engine.execute('review', { ...TRIGGER } as never); + + const first = await h.engine.resume(paused.runId!, decided('approve', 'usr_mgr')); + expect('impersonatedBy' in h.ctxOf('after1')).toBe(false); + expect(first.status).toBe('paused'); + + const h2 = envelopeHarness(); + h2.engine.registerFlow('review', reviewFlow('review', { recorder: true }) as never); + const p2 = await h2.engine.execute('review', { ...TRIGGER } as never); + const mid = await h2.engine.resume(p2.runId!, decidedImpersonated('approve', 'usr_mgr', 'usr_admin')); + expect(h2.ctxOf('after1').impersonatedBy).toBe('usr_admin'); + await h2.engine.resume(mid.runId ?? p2.runId!, decided('approve', 'usr_dir')); + const after2 = h2.ctxOf('after2'); + expect(after2.userId).toBe('usr_dir'); + expect('impersonatedBy' in after2, "the second decider acted in their own name").toBe(false); + }); + + it('CONTROL: a signal WITHOUT the owning service\'s marker cannot name an administrator', async () => { + const h = envelopeHarness(); + h.engine.registerFlow('review', reviewFlow('review', { recorder: true, gate: 'open_gate' }) as never); + const paused = await h.engine.execute('review', { ...TRIGGER } as never); + + // The shape a JSON body could produce: no symbol, so neither key is honoured. + await h.engine.resume(paused.runId!, { branchLabel: 'approve', output: { decision: 'approve' }, resumedBy: 'usr_mgr', impersonatedBy: 'usr_admin' }); + const after = h.ctxOf('after1'); + expect(after.userId).toBe('usr_rep'); + expect('impersonatedBy' in after).toBe(false); + }); + + it('CONTROL: an administrator named with no decider beside it changes nothing', async () => { + const h = envelopeHarness(); + h.engine.registerFlow('review', reviewFlow('review', { recorder: true }) as never); + const paused = await h.engine.execute('review', { ...TRIGGER, impersonatedBy: 'usr_admin' } as never); + + await h.engine.resume(paused.runId!, { branchLabel: 'approve', output: { decision: 'approve' }, impersonatedBy: 'usr_admin2', [RESUME_AUTHORITY_SERVICE]: true }); + const after = h.ctxOf('after1'); + expect(after.userId, 'no human decided: the run keeps its identity').toBe('usr_rep'); + expect(after.impersonatedBy, 'and its own stamp').toBe('usr_admin'); + }); +}); + describe('on the real stack: attribution after the decision, and the refusal when the decider lacks a right', () => { let kernel: ObjectKernel; let ql: ObjectQL; diff --git a/packages/services/service-automation/src/run-resume.ts b/packages/services/service-automation/src/run-resume.ts index 2090f54fcc1..a7ed5ba3538 100644 --- a/packages/services/service-automation/src/run-resume.ts +++ b/packages/services/service-automation/src/run-resume.ts @@ -325,7 +325,19 @@ async function deciderSegmentContext( const context = run.context; const decider = signal[RESUME_AUTHORITY_SERVICE] === true ? signal.resumedBy : undefined; if (typeof decider !== 'string' || decider === '') return context; - if (context?.userId === decider) return context; + // The decider's segment names exactly the DECISION's impersonating + // administrator: the one the owning service read off the decision + // request's session, honoured only beside the decider it vouches for. + // The stamp of the write that STARTED the run is never carried over: + // whoever was at the keyboard then did not make this decision. + const impersonatedBy = decisionImpersonatorOf(signal); + if (context?.userId === decider) { + if (context.impersonatedBy === impersonatedBy) return context; + const same: AutomationContext = { ...context }; + if (impersonatedBy) same.impersonatedBy = impersonatedBy; + else delete same.impersonatedBy; + return same; + } const switched: SeededRunContext = { ...(context ?? {}), userId: decider, @@ -339,9 +351,10 @@ async function deciderSegmentContext( // segment the deployment-level rows. Seeded for the decider below. delete switched.posture; // The impersonating administrator belonged to the write that STARTED the - // run: the decider is a different person acting in their own name, so the - // segment they own names no impersonator. + // run: the segment the decider owns names only the administrator, if any, + // who made THIS decision while impersonating the decider. delete switched.impersonatedBy; + if (impersonatedBy) switched.impersonatedBy = impersonatedBy; // The trigger user's email is theirs too: kept under the decider's id, // it would admit the decider to every row an email-keyed policy grants // the submitter. Removed on both postures, and seeded for the decider @@ -353,6 +366,19 @@ async function deciderSegmentContext( return switched; } +/** + * The administrator a decision's resume signal names as impersonating its + * decider ({@link ResumeSignal.impersonatedBy}), as a non-blank string, or + * `undefined`. Read only by {@link deciderSegmentContext}, after the signal's + * service marker and decider are established. + */ +function decisionImpersonatorOf(signal: ResumeSignal): string | undefined { + const by = signal.impersonatedBy; + if (typeof by !== 'string') return undefined; + const trimmed = by.trim(); + return trimmed ? trimmed : undefined; +} + /** The body of {@link AutomationEngine.expandDeclaredLookups}, moved unedited; that method's docblock is its contract. */ export async function expandDeclaredLookups(this: Pick, flow: FlowParsed, runContext: AutomationContext): Promise { if (!this.recordExpander) return; diff --git a/packages/spec/src/contracts/automation-service.ts b/packages/spec/src/contracts/automation-service.ts index 9fe3719c577..1fb0bfc9309 100644 --- a/packages/spec/src/contracts/automation-service.ts +++ b/packages/spec/src/contracts/automation-service.ts @@ -310,9 +310,12 @@ export interface AutomationContext { * * ATTRIBUTION ONLY: {@link userId} stays the impersonated user, whose * grants a `runAs:'user'` run resolves, and nothing in the authorization - * path reads this. Absent on every run a user's own write started. Cleared - * when an approval decision hands the run to the decider, who is not - * impersonated. + * path reads this. Absent on every run a user's own write started. An + * approval decision replaces it for the decider's segment with the + * decision's own {@link ResumeSignal.impersonatedBy}: the administrator + * when the decider decided from an impersonation session, absent when the + * decider acted in their own name, never the stamp of the write that + * started the run. */ impersonatedBy?: string; /** Additional contextual data */ @@ -875,6 +878,23 @@ export interface ResumeSignal { * cannot carry that marker, so a body naming a user here changes nothing. */ resumedBy?: string; + /** + * The administrator (a `sys_user` id) at the keyboard when the decider + * named by {@link resumedBy} decided from an impersonation session — the + * decision request's `ExecutionContext.impersonatedBy`, as the owning + * service read it off that request's session. The nodes after the + * decision carry it as the segment's + * {@link AutomationContext.impersonatedBy}, so the audit writer names the + * administrator on the decider's writes as on the decision itself; absent + * when the decider acted in their own name, and then the segment carries + * none, whatever the run's trigger carried. + * + * ATTRIBUTION ONLY: {@link resumedBy} stays the decider whose identity the + * segment runs with. Honoured ONLY together with {@link resumedBy} on a + * signal that also carries {@link RESUME_AUTHORITY_SERVICE}; a signal built + * from an HTTP body naming an administrator here changes nothing. + */ + impersonatedBy?: string; /** * Set by the service that OWNS the suspension to clear the resume gate on * a `resumeAuthority: 'service'` node — or on one that declares no