diff --git a/AGENT_IDENTITY_WRITEUP.md b/AGENT_IDENTITY_WRITEUP.md index 2962496dd68..41f06f1b3d3 100644 --- a/AGENT_IDENTITY_WRITEUP.md +++ b/AGENT_IDENTITY_WRITEUP.md @@ -1,23 +1,34 @@ # Agent Identity — Build vs. Plan (first notes) -> Draft / working notes, not the final readout. Compares what we actually built against -> the 2-day hackathon plan: what matched, what we changed and why, what we skipped, and -> what we learned. Branch: `afitzek/track-2-implementation`. +> Working notes, not the final readout. This compares what we actually built against the +> 2-day hackathon plan: what matched, what we changed and why, what we skipped, and what we +> learned. Branch: `afitzek/track-2-implementation`. +> +> **Quick glossary.** *SA* = service account (a user row that is not a person). *OBO* = +> on-behalf-of (an agent acting for the human who triggered it). *AS* = authorization server +> (here, n8n's own OAuth server). *PRM* = protected-resource metadata (RFC 9728). ## TL;DR -We proved the headline the plan set out to prove — **an internal construct (an Agent) can -hold a real, verifiable identity and cross a real n8n-owned boundary as itself** — and it's -green end-to-end (proven both in-process and over real HTTP; runbook §4). Along the way one -"free" wiring the plan assumed turned out to be a trap, so Track 3 took a different shape -than written, and we went *past* the plan on the protocol side (RFC 9728/8414 discovery, -audience scoping). The single most novel idea in the plan — **on-behalf-of-a-human delegated -tokens** — is still not built, and the repo now contains a constraint that makes building it -a design decision rather than an extension (see [Gaps](#gaps-we-wanted-to-research-but-didnt-reach)). +We proved the main thing the plan set out to prove: **an internal construct (an Agent) can +hold a real, checkable identity and cross a real n8n-owned boundary as itself.** It works end +to end — proven both in-process and over real HTTP (runbook §4). -One **security finding** exists only because the tracks were built separately: `disabled` is -the only kill switch a service account has, and Track 2/3's machine path never reads it — -see [Where the tracks meet](#where-the-tracks-meet). +Three things changed along the way: + +- One wiring the plan called "free" turned out to be a trap, so Track 3 took a different shape + than written (see [The one assumption that broke](#the-one-assumption-that-broke)). +- We went **past** the plan on the protocol side: the mint now discovers where to get its token + using standard OAuth metadata (RFC 9728 → 8414), and tokens are audience-scoped (RFC 8707). +- The plan's most ambitious idea — **on-behalf-of-a-human delegated tokens** — is now built for + the **internal** case (RFC 8693-correct, proven by GATE-OBO): a human-triggered agent run + mints a token that says `{sub: human, act: SA}`, and the boundary authorizes **as the human**. + Delegation for humans coming from an *outside* identity provider is the remaining slice + (see [Delegated tokens](#delegated--on-behalf-of-tokens--internal-slice-built-this-session)). + +There is also one **security finding** that only exists because the tracks were built +separately: `disabled` is the only "off switch" a service account has, and the Track 2/3 +machine path never checks it — see [Where the tracks meet](#where-the-tracks-meet). --- @@ -25,9 +36,9 @@ see [Where the tracks meet](#where-the-tracks-meet). | Track | Plan intent | What we built | Verdict | |---|---|---|---| -| **1 — Service-account User** | Mark a `User` as SA; impersonation as the "what can it touch" tool | Dedicated `User.type='serviceAccount'` column (not the password-null pending shape); passwordless + personal project; impersonation with an `act` claim | Built as intended (pre-session) | -| **2 — Inbound M2M auth** | `client_credentials`, token endpoint, verify into MCP Trigger + public API + triggers | All of it, **plus** RFC 8707 resource/audience scoping (`aud` claim, audience isolation) — which the plan explicitly put *out* of scope | Built + **extended** (pre-session) | -| **3 — Agent ↔ SA** | 1:1 SA per agent; wire SA into `AgentsCredentialProvider.user`; agent mints its own token → MCP Trigger | 1:1 SA per agent ✓; **did NOT** wire the credential provider (see broken assumption); a new `n8nInternalOAuth2` marker credential that mints **per tool call** via `client_credentials`, endpoint **discovered** via RFC 9728→8414 | Built, **reshaped** (this session) | +| **1 — Service-account User** | Mark a `User` as an SA; use impersonation as the "what can it touch" tool | A dedicated `User.type='serviceAccount'` column (not the password-null "pending" shape); passwordless, with its own project; impersonation carries an `act` claim | Built as intended (pre-session) | +| **2 — Inbound M2M auth** | `client_credentials`, a token endpoint, verify it in the MCP Trigger + public API + triggers | All of that, **plus** RFC 8707 audience scoping (an `aud` claim, so a token only works at one URL) — which the plan had put *out* of scope | Built + **extended** (pre-session) | +| **3 — Agent ↔ SA** | One SA per agent; wire that SA into `AgentsCredentialProvider.user`; agent mints its own token → MCP Trigger | One SA per agent ✓; **did not** wire the credential provider (see the trap below); a new `n8nInternalOAuth2` marker credential that mints a token **per tool call**, finding the endpoint via RFC 9728→8414 discovery | Built, **reshaped** (this session) | --- @@ -36,50 +47,52 @@ see [Where the tracks meet](#where-the-tracks-meet). Full implementation log: [Service Accounts POC: implementation, then two impersonation bugs](https://app.notion.com/p/n8n/Service-Accounts-POC-implementation-then-two-impersonation-bugs-3b15b6e0c94f81d5b233e50b357409a5). -**The model.** A service account is a row in the same `user` table, with the same roles, -scopes, credential ownership and project membership. The only difference is that it isn't a -person: no password ever, no login, managed entirely through impersonation. Nothing -downstream — RBAC, credential sharing, project scoping — had to be taught anything new. +**The model.** A service account is a row in the same `user` table as a person, with the same +roles, scopes, credential ownership, and project membership. The only difference: it is not a +person. It has no password, can't log in, and is driven entirely through impersonation. Nothing +downstream — RBAC, credential sharing, project scoping — had to learn anything new. -**`password: null`, not token-exchange's `INVALID_PASSWORD_PLACEHOLDER`.** That sentinel -exists because those users *are* people who should look fully set up; we want the opposite — -an SA should be invisible to every onboarding path. `null` gets that free from filters that -already existed: password reset can't find it (`user.repository.ts:86`, the -`password: Not(IsNull())` at `:92`), no invite mail is sent, login compares against null. -Cost is two one-line fixes — `computeIsPending` (`user.ts:125`) and the pending-users SQL -(`user.repository.ts:320`). +**Why `password: null` (and not token-exchange's `INVALID_PASSWORD_PLACEHOLDER`).** That +placeholder exists to make federated users *look* fully set up. We want the opposite: an SA +should be invisible to every onboarding path. `null` gets that for free from filters that +already exist — password reset can't find it (`user.repository.ts:86`, the +`password: Not(IsNull())` at `:92`), no invite email goes out, and login compares against +null. The whole cost was two one-line fixes: `computeIsPending` (`user.ts:125`) and the +pending-users query (`user.repository.ts:320`). -**Separate `serviceAccount:*` scopes, not `user:*`** (`global-scopes.ee.ts:59–64`). -`user:list` is already granted to every member (`:188`), so reusing it would leak SAs; -`user:create` carries seat/invite/email meaning an SA has none of; and impersonation has to -be grantable and revocable on its own. +**Why separate `serviceAccount:*` scopes instead of reusing `user:*`** (`global-scopes.ee.ts:59–64`). +Every member already has `user:list` (`:188`), so reusing it would leak SAs to everyone. +`user:create` implies seats/invites/email that an SA has none of. And impersonation needs to be +granted and revoked on its own. -**`password IS NULL` is used all over as shorthand for "a real person", and an SA breaks it -both ways** — it reads as half-set-up where it should be active, and inactive where it should -be visible. Swept in `buildUserQuery` (`user.repository.ts:411`, now defaults to -`type = 'user'`), `computeIsPending`, `countUsersByRole` (`:103`, feeds seat telemetry), -`findEligibleByProjectOrGlobalRoles` (`:187`), and three predicates in `project.repository.ts` -collapsed into one `CREATOR_IS_ACTIVATED` constant (`:15`). +**`password IS NULL` is used all over the code as shorthand for "a real person" — and an SA +breaks that reading both ways.** It reads as half-set-up where the SA should look active, and +as inactive where it should be visible. We swept those spots: `buildUserQuery` +(`user.repository.ts:411`, now defaults to `type = 'user'`), `computeIsPending`, +`countUsersByRole` (`:103`, which feeds seat telemetry), `findEligibleByProjectOrGlobalRoles` +(`:187`), and three predicates in `project.repository.ts` collapsed into one +`CREATOR_IS_ACTIVATED` constant (`:15`). -**The guard that mattered most.** Invite-acceptance gated on `if (invitee.password) throw`. -An SA has none, so it **passed** — the endpoint would have handed it a password and a session -cookie, turning it into a human login that keeps its role and its credentials. Now blocked -before the password check (`invitation.controller.ts:126`), with the same guard on `me` -(`me.controller.ts:58,206,274,325`), all four MFA enrolment routes -(`mfa.controller.ts:70,82,127,214`) and impersonation entry (`impersonation.controller.ts:36`). +**The guard that mattered most.** Invite-acceptance was gated on `if (invitee.password) throw`. +An SA has no password, so it **passed** the guard — the endpoint would have handed it a +password and a session cookie, turning it into a human login that keeps the SA's role and +credentials. That's now blocked *before* the password check (`invitation.controller.ts:126`), +with the same guard on `me` (`me.controller.ts:58,206,274,325`), all four MFA-enrolment routes +(`mfa.controller.ts:70,82,127,214`), and impersonation entry (`impersonation.controller.ts:36`). -**The lockout bug.** Both impersonation transitions revoke the cookie they replace, and a -session token's only varying field is `iat`, in whole seconds — so leaving impersonation -within a second of entering it re-minted a **byte-identical** token that was already revoked, -locking the operator out of their own session. Fixed with a random `jti` on transition tokens -only (`auth.service.ts:74,334,343`), leaving normal session tokens unchanged. +**The lockout bug.** Both impersonation transitions revoke the cookie they replace. A session +token's only changing field is `iat`, measured in whole seconds — so leaving impersonation +within one second of entering it re-minted a **byte-identical** token that had just been +revoked, locking the operator out of their own session. Fixed by adding a random `jti` to +*transition* tokens only (`auth.service.ts:74,334,343`); normal session tokens are unchanged. -**Two things that were blockers, not polish.** `isWithinUsersLimit()` is false on *any* -seat-capped licence, so without exempting SAs every non-owner cookie issuance throws -`USERS_QUOTA_REACHED` and impersonation never starts (`auth.service.ts:302–315`). And the MFA -gate reads `user.mfaEnabled`, always false on an SA, so an MFA-enforced instance would 401 the -session and push it to an enrolment screen it can never complete — fixed by gating on the -human instead (`auth.service.ts:401–420`, actor-hash binding at `:484–496`). +**Two things that were outright blockers, not polish.** First, `isWithinUsersLimit()` returns +false on *any* seat-capped licence, so without exempting SAs, issuing any non-owner cookie +throws `USERS_QUOTA_REACHED` and impersonation can never start (`auth.service.ts:302–315`). +Second, the MFA gate reads `user.mfaEnabled`, which is always false on an SA — so on an +MFA-enforced instance the SA session would 401 and get pushed to an enrolment screen it can +never complete. Fixed by checking MFA on the *human* instead (`auth.service.ts:401–420`, with +actor-hash binding at `:484–496`). --- @@ -89,215 +102,400 @@ human instead (`auth.service.ts:401–420`, actor-hash binding at `:484–496`). |---|---|---| | External system → n8n REST API | Yes (Track 2) | ✅ Yes | | Webhook-triggered external call | Yes (Track 2) | ✅ Yes | -| **Agent proves its own identity via an MCP Trigger** | Yes (Track 3 proof) | ✅✅ **Yes — the headline, proven in-process and over real HTTP (runbook §4)** | -| Agent/workflow → third-party API via stored creds, *attribution improved* | Yes (wire SA into `AgentsCredentialProvider`) | ❌ **No — we deliberately left the provider untouched.** This scenario is a gap. | +| **Agent proves its own identity via an MCP Trigger** | Yes (Track 3 proof) | ✅✅ **Yes — the headline. Proven in-process and over real HTTP (runbook §4).** | +| Agent/workflow → third-party API via stored creds, *attribution improved* | Yes (wire SA into `AgentsCredentialProvider`) | ❌ **No — we deliberately left the provider untouched.** A gap. | | Agent → another workflow directly as a tool | No | No (unchanged, by design) | | Two n8n instances calling each other | No | No | | Agent ↔ external/cross-org agent | No | No | -| Agent on behalf of a specific person | No (stretch) | No — still autonomous-only | +| Agent on behalf of a specific person | No (stretch) | ✅ **Internal built — a human-triggered chat run mints `{sub: human, act: SA}`, and the boundary authorizes as the human (GATE-OBO). External-IdP delegation is still ahead.** | --- ## The one assumption that broke **Plan (Track 3):** *"Wire that service-account user into `AgentsCredentialProvider`'s `user` -param… Confirmed free: the provider takes a plain `User`… No provider changes needed, only -what each caller passes."* +param… Confirmed free: the provider takes a plain `User`… No provider changes needed, only what +each caller passes."* -Type-wise that was true. **Semantically it was a trap.** A service account lives in *its own -personal project*, so `getCredentialsAUserCanUseInAWorkflow(agentSA, {projectId})` intersects -the SA's (empty) credential set with the agent's project → **∅**. Passing the SA into the -provider would have *removed* the agent's credential access, not governed it — a regression, -worst exactly on the autonomous runtime paths that matter most. +The types lined up, so it looked free. **But it was a semantic trap.** A service account lives +in *its own* project. So `getCredentialsAUserCanUseInAWorkflow(agentSA, {projectId})` computes +the overlap between the SA's credentials (none, in the agent's project) and the agent's project +credentials → **empty set**. Passing the SA into the provider wouldn't have *governed* the +agent's credential access — it would have **removed** it. And it would have hurt most on the +autonomous runtime paths, which matter most. -**Consequence / reinterpretation:** we separated two axes the plan conflated — -- *identity that authorizes credential access* (left as-is: project scope), and -- *identity that a minted token represents* (the new bit: the agent's SA, threaded only for minting). +**What we did instead:** we split apart two ideas the plan had merged into one — -So "the agent acts as itself" is realized through **token minting**, not through -credential-provider scoping. This is arguably a cleaner model, but it means the plan's -"attribution improved for third-party credential use" was **not delivered**. +- the identity that **authorizes credential access** (left as-is: project scope), and +- the identity that a **minted token represents** (the new part: the agent's SA, threaded only + for minting). + +So "the agent acts as itself" happens through **token minting**, not through +credential-provider scoping. It's arguably a cleaner split — but it means the plan's promise of +"better attribution for third-party credential use" was **not delivered**. --- ## Where the tracks meet -Tracks 1 and 2/3 were built separately against the same account. These four properties exist -only *between* them, and none is visible from inside either track. +Tracks 1 and 2/3 were built separately against the same account. These four properties only +exist *between* the tracks, and none of them is visible from inside either one. -### 1. `disabled` is a kill switch for the human path only +### 1. `disabled` is an off switch for the human path only ⚠️ The one item here worth fixing in code. -Each decision was right on its own. Track 1 chose `password: null`, which makes -`createJWTHash` permanently constant for an SA — so hash-based revocation does nothing to it, -as the code itself says (`auth.service.ts:486–491`). That leaves `disabled` as the **only** -kill switch an SA has. Track 2/3 then added a second way in that never reads it. +Each decision was fine on its own. Track 1 chose `password: null`, which makes `createJWTHash` +permanently constant for an SA — so hash-based revocation does nothing to it, as the code +itself notes (`auth.service.ts:486–491`). That leaves `disabled` as the SA's **only** off +switch. Then Track 2/3 added a second way in that never checks it. -**Enforced** (all on the human path): `auth.service.ts:465` (the impersonated account), `:508` -(the human behind it), `impersonation.controller.ts:50` (entering impersonation) and `:132` +**Checked** (all on the human path): `auth.service.ts:465` (the impersonated account), `:508` +(the human behind it), `impersonation.controller.ts:50` (entering impersonation), and `:132` (restoring the session afterwards). -**Not enforced** (machine path): `exchangeClientCredentials` never loads the `User` at all +**Not checked** (machine path): `exchangeClientCredentials` never loads the `User` at all (`oauth-server.service.ts:411–464`) — it checks the client secret, then mints against -`credential.userId`. Verify does load the `User` but only checks that it *exists* -(`oauth-token.service.ts:312`) before `resource.authorize(user)` (`:322`). +`credential.userId`. Verify does load the `User`, but only checks that it *exists* +(`oauth-token.service.ts:312`) before calling `resource.authorize(user)` (`:322`). -**So:** disabling a service account stops a person impersonating it, and does **not** stop it -minting new tokens or using ones it already holds. The UI switch -(`service-accounts.service.ts:148`) looks like a kill switch and isn't one. +**In plain terms:** disabling a service account stops a person from impersonating it, but does +**not** stop it from minting new tokens or using ones it already holds. The UI toggle +(`service-accounts.service.ts:148`) looks like a kill switch but isn't one. -- **Minimum fix:** reject on `disabled` in `exchangeClientCredentials`, and at +- **Minimum fix:** reject when `disabled` in `exchangeClientCredentials`, and at `oauth-token.service.ts:312`. - **Better fix:** a `sessionEpoch` counter on `user`, folded into `createJWTHash` and checked at verify — bumping it revokes everything for that account. Worth having for humans too, and - it also covers §4. + it also fixes §4. ### 2. Two ways into one account, not equally tight -Track 1 gave the SA no password *specifically* so that no secret could ever authenticate as -it — the only way in was a human already holding `serviceAccount:impersonate`. Track 2/3 gives -it a reversibly-encrypted `clientSecret` that does exactly that, from anywhere, with no human -involved. +Track 1 gave the SA no password *on purpose*, so that no secret could ever authenticate as it — +the only way in was a human already holding `serviceAccount:impersonate`. Track 2/3 then gave +it a reversibly-encrypted `clientSecret` that does exactly that: authenticate as the SA, from +anywhere, with no human involved. -Not wrong — n8n has to present the secret outbound (decision 4) — but it retires the guarantee -the `password: null` choice was made for, so it belongs next to the at-rest-downgrade note in -`SERVICE_ACCOUNT_PROOF.md` § *Security notes*, not only in the Track 3 rationale. +That's not wrong — n8n has to present the secret outbound (see decision 4) — but it quietly +retires the very guarantee the `password: null` choice was made for. So it belongs next to the +at-rest-downgrade note in `SERVICE_ACCOUNT_PROOF.md` § *Security notes*, not only in the Track 3 +rationale. ### 3. The newer path is narrow; the older one is wide -An OAuth token only works against the exact URL it was minted for. The impersonation cookie -has no such restriction — it satisfies `validateCookieToken` (`auth.service.ts:358`) and +An OAuth token only works against the exact URL it was minted for. The impersonation cookie has +no such limit — it satisfies `validateCookieToken` (`auth.service.ts:358`) and `authenticateUserByCookie` (`:391`) unconditionally, so anything reachable by cookie inherits -impersonation without having been reviewed for it: `webhook-helpers.ts:574` and +impersonation without ever being reviewed for it: `webhook-helpers.ts:574` and `n8n-identifier.ts:66`. Worth remembering when webhook auth comes back into scope. ### 4. Neither path can cancel a single session **Cookie side:** hash revocation does nothing for an SA (see §1), so it only works through the -*human's* hash — they change password or rotate MFA and their impersonation sessions die. -Coarse, but real. **Machine side:** no revocation list for minted tokens and no `disabled` -check, so a leaked token is good until it expires; rotating the client secret doesn't touch -tokens already issued. The `sessionEpoch` idea in §1 covers both. +*human's* hash — they change their password or rotate MFA, and their impersonation sessions +die. Coarse, but real. **Machine side:** there's no revocation list for minted tokens and no +`disabled` check, so a leaked token is good until it expires, and rotating the client secret +doesn't touch tokens already issued. The `sessionEpoch` idea in §1 fixes both. --- ## Assumptions confirmed / extended -- **Confirmed:** `oauth-server` had `authorization_code`+DCR but **no** `client_credentials` — it was net-new (Track 2). "MCP APIs" = the **MCP Trigger node** auth path (not the instance MCP server). SA discriminator = a dedicated `type` column (chosen over reusing password-null). -- **Extended beyond plan:** resource/audience scoping was built (plan said "a plain token is enough"). And we added a whole **protocol-discovery layer** the plan never mentioned: the mint now discovers its token endpoint + canonical resource from n8n's *own* RFC 9728 / 8414 metadata instead of hardcoding. The guiding contract: +- **Confirmed:** `oauth-server` already had `authorization_code` + Dynamic Client Registration, + but **no** `client_credentials` — that was new in Track 2. "MCP APIs" meant the **MCP Trigger + node** auth path, not the instance MCP server. The SA discriminator is a dedicated `type` + column (chosen over reusing password-null). +- **Extended beyond the plan:** we built audience scoping (the plan said "a plain token is + enough"), and we added a whole **protocol-discovery layer** the plan never mentioned — the + mint now discovers its token endpoint and canonical resource from n8n's *own* RFC 9728 / 8414 + metadata instead of hardcoding them. The guiding rule: - > The auth transport and discovery are pure OAuth2 (RFC 9728 → 8414 → 8707/6749) with no - > n8n-invented handshake; the identity model and the credential↔(AS, resource) binding are + > The auth transport and discovery are pure OAuth2 (RFC 9728 → 8414 → 8707/6749), with no + > n8n-invented handshake. The identity model and the credential↔(AS, resource) binding are > n8n domain data that OAuth2 deliberately doesn't own. --- -## Design decisions — what, why, right? +## Design decisions — what, why, was it right? -1. **Leave `AgentsCredentialProvider` untouched.** Why: the ∅ trap. Right? Yes — avoided breaking every agent run; kept identity-for-mint and identity-for-authz as separate concerns. -2. **Identity-less marker credential** (`n8nInternalOAuth2`) that resolves the *acting* SA's credential from execution context, rather than storing a secret or hard-referencing one SA. Why: generalizes ("also works for authenticated users"), keeps the node dumb. Right? Yes — it's the seam that makes per-AS credentials a future config change, not a redesign. -3. **Real HTTP `client_credentials` self-call**, not in-process mint. Why: exercises the actual boundary (the whole point of the proof). Right? Yes for proof value — but it *forced* decision 4. -4. **Reversible `clientSecret` (bcrypt → n8n `Cipher`).** Why: an HTTP mint needs to *recover* the secret; bcrypt is one-way. Right? Defensible — n8n must present the secret outbound anyway, and this matches how *all* n8n credentials are stored (instance key already the crown jewel). Documented as a deliberate at-rest tradeoff; inbound verify stays constant-time. See also [Where the tracks meet §2](#2-two-ways-into-one-account-not-equally-tight) — it also relaxes a Track 1 guarantee. -5. **Audience auto-derived → then discovered** (canonical `resource` from PRM). Why: robustness + protocol-honesty. Right? Yes — also fixed a latent bug (we were guessing `/oauth/token`; the AS actually advertises `/mcp-oauth/token`). -6. **Eager provisioning on agent create + lazy backfill.** Right? Yes — flag-gated, non-breaking when off. -7. **Audit rows on mint + verify** (stretch goal #2). Right? Yes — high demo value, reused `EventService`, near-free. Covers the *token* lifecycle only; see the attribution gap in [Known rough edges](#known-rough-edges-track-1-follow-ups-still-open). +1. **Leave `AgentsCredentialProvider` untouched.** Why: the empty-set trap. Right? Yes — it + avoided breaking every agent run, and kept "identity for minting" and "identity for + authorizing credential access" as separate concerns. +2. **An identity-less marker credential** (`n8nInternalOAuth2`) that finds the *acting* SA's + credential from execution context, instead of storing a secret or pointing at one fixed SA. + Why: it generalizes ("also works for authenticated users") and keeps the node dumb. Right? + Yes — it's the seam that makes per-AS credentials a future config change rather than a + redesign. +3. **A real HTTP `client_credentials` self-call**, not an in-process mint. Why: it exercises the + actual boundary, which is the whole point of the proof. Right? Yes for proof value — but it + *forced* decision 4. +4. **A reversible `clientSecret` (bcrypt → n8n `Cipher`).** Why: an HTTP mint has to *recover* + the secret to send it, and bcrypt is one-way. Right? Defensible — n8n has to present the + secret outbound anyway, and this matches how *all* n8n credentials are stored (the instance + key is already the crown jewel). It's a deliberate at-rest tradeoff; inbound verification + still uses a constant-time compare. See [Where the tracks meet §2](#2-two-ways-into-one-account-not-equally-tight) + — it also relaxes a Track 1 guarantee. +5. **Audience auto-derived, then discovered** (canonical `resource` from PRM). Why: robustness + plus protocol-honesty. Right? Yes — it also fixed a real bug: we were guessing `/oauth/token`, + but the AS actually advertises `/mcp-oauth/token`. +6. **Eager provisioning on agent create + lazy backfill.** Right? Yes — it's flag-gated and + non-breaking when the flag is off. +7. **Audit rows for the whole identity story** (started as stretch goal #2). Right? Yes — high + demo value, reuses `EventService`, near-free. It began as token mint/verify only; this + session extended it to record both subject and actor, added a server-side event for + delegation, and wired the account/impersonation lifecycle events that were previously + emitted but never routed anywhere. See [Audit trail](#audit-trail). +8. **On-behalf-of authorizes as the subject (the human), not the actor.** Why: the agent acts + *as* the human who triggered it; the SA is kept in `act` for attribution. This is + deliberately a *third* answer to "who's in charge" — not `actor ?? subject` — and + deliberately *not* routed through `req.tokenGrant`, which would silently swap authority (see + [Delegated tokens](#delegated--on-behalf-of-tokens--internal-slice-built-this-session)). + Right? Yes — it matches the intent of delegation and dodges the documented trap; GATE-OBO + proves authorization follows the human. +9. **`act` comes from the `actor_token`, not from client auth (RFC 8693).** Why: a first cut + derived the actor from `client_id`, which merged the OAuth *client* with the RFC *actor* and + limited actors to SAs. Right? Yes — a two-step mint (the SA gets its own token, then presents + it as `actor_token`) generalizes to non-SA actors and is RFC-compatible. Corrected mid-build + after review. + +--- + +## Audit trail + +Every step of the identity story now writes an audit event, through the existing pipeline +(`EventService` → the log-streaming relay → `sendAuditEvent`, i.e. the standard audit +destination — no custom logger). All payloads carry **identifiers only** — user ids, client id, +audience, outcome — never secrets or token strings. Where delegation applies, the event records +both the **subject** (`sub`, the human) and the **actor** (`act`, the SA). + +**Token lifecycle (3 events).** + +| Event | Payload | When | +|---|---|---| +| `n8n.audit.service-account.token.minted` | `{ sub, act?, clientId, aud, outcome }` | The internal mint service issues a token. Autonomous → `sub = SA`, no `act`. Delegated → `sub = human`, `act = SA`. | +| `n8n.audit.service-account.token.exchanged` | `{ sub, act, clientId, aud, outcome }` | The token-exchange grant runs at the AS itself (server-side), so delegation is auditable even for callers that don't go through the mint service. | +| `n8n.audit.service-account.token.verified` | `{ sub, act?, aud, outcome }` | A protected resource verifies an inbound token. `act` is the token's actor, when present. | + +**Account & impersonation lifecycle (5 events).** These were already emitted, but no relay +consumed them, so they never reached the audit log. That's now fixed. Each carries the **human +actor** (`userId`) and the SA (`serviceAccountId`): +`n8n.audit.service-account.created`, `.role-changed`, `.deleted`, `.impersonation.started`, +`.impersonation.ended`. + +**One nuance to know.** The `minted` event fires only for tokens that go through the internal +mint service (the agent path). A direct external `client_credentials` call to `/oauth/token` +gets a token but no `minted` row. Verification and exchange are audited for everyone, because +they happen server-side. For a delegated mint, the client-side `minted` and the server-side +`exchanged` both fire on purpose (one records "the agent asked", the other "the AS issued"), +and they line up with the `verified` event by their shared `jti` (a correlation id logged at +mint and verify). + +### Seeing the trail: the DB sink (PoC) + +Audit events travel over the MessageEventBus, so any log-streaming destination can receive +them. For the demo we added a **database sink**, so the trail is visible inside the instance +without having to wire up an external destination. This is what makes the whole audit story +demoable: mint, exchange, verify, and the account/impersonation lifecycle all land in one +queryable table. + +- **Opt-in via `N8N_AUDIT_LOG_DB_SINK=true`.** On boot, the log-streaming module adds an + in-memory destination — `MessageEventBusDestinationDatabase`, subscribed to `*` + (`log-streaming.module.ts:32`) — that writes every event it receives into a new + `audit_log_event` table (migration `1785900000001`). It's recreated on every boot and never + saved to the destinations config, so it needs no serialize/deserialize. +- **The table** (`AuditLogEvent`): `id` (the originating EventMessage uuid — reused as the + primary key, so re-delivery is idempotent), `eventName` (indexed), `message`, `ts` (the + event's *own* timestamp, kept separate from `createdAt`, which is ingest time), and `payload` + (JSON). The destination's `anonymizeAuditMessages` flag is honored. +- **Browsing it:** `GET /rest/eventbus/audit-log-events` (scope `eventBusDestination:list`; not + license-gated, unlike the streaming-destination endpoints), paginated, with an optional + `prefix` filter — e.g. `prefix=n8n.audit.service-account` shows just the identity events. A + settings page (`AuditLogView.vue` + `auditLog.store.ts`) renders it as a filterable, + server-paginated table. +- **PoC caveats, called out in the code:** the sink subscribes to **all** events (`*`), not + only `n8n.audit.*`, so filter by prefix to see the audit ones; the table is **unbounded, with + no pruning**; each event is a separate `save` (no batching); and the store/page are + deliberately lightweight (the store isn't even in the `STORES` enum). Fine for a demo, not for + production retention. --- ## Shortcuts we took (and the upgrade path) -- **Provisioning isn't atomic, and the transaction style is a known review blocker.** - `ServiceAccountsService.create/.delete` open their own transactions and take no `ctx`, so - WP2 uses **compensation** (tear down the SA if a later step fails) instead of one enclosing +- **Provisioning isn't atomic, and its transaction style is a known review blocker.** + `ServiceAccountsService.create/.delete` open their own transactions and take no `ctx`, so WP2 + uses **compensation** (tear the SA down if a later step fails) instead of one enclosing transaction. They also do it via `this.userRepository.manager.transaction(...)` (`service-accounts.service.ts:114,232`) — reaching into `.manager` from a *service*, which - `AGENTS.md` names as a reviewer-rejected anti-pattern. It isn't lint-enforced (the rule - catches the *import*, not `.manager`), so it passed CI. One fix covers both: accept a `ctx` - and inject `TransactionRunner`, as `oauth-token.service.ts:62` already does. -- **Discovery fallback + no caching + `ssrf:'disabled'`.** If discovery fails we fall back to the hardcoded endpoint; metadata is re-fetched every tool call; SSRF is disabled because the target is the same instance. Fine for an internal PoC. → cache metadata (CacheService); flip SSRF for external. -- **Credential selection is fixed to the acting SA's internal cred.** No per-AS credential binding yet (in-code TODO). -- **Three security items deferred as in-code TODOs:** (a) confused-deputy guard — validate discovered `resource` == intended target; (b) credential selection keyed on discovered AS issuer; (c) external-AS SSRF/issuer-allowlist. A fourth is now on the list: the `disabled` check on the machine path ([Where the tracks meet §1](#1-disabled-is-a-kill-switch-for-the-human-path-only)). -- **No node `typeVersion` bump** — the new auth option is a backward-compatible enum add. + `AGENTS.md` calls a reviewer-rejected anti-pattern. Lint doesn't catch it (the rule catches + the *import*, not `.manager`), so it passed CI. One fix covers both: accept a `ctx` and inject + `TransactionRunner`, the way `oauth-token.service.ts:62` already does. +- **Discovery: fallback, no caching, `ssrf:'disabled'`.** If discovery fails we fall back to the + hardcoded endpoint; metadata is re-fetched on every tool call; SSRF is disabled because the + target is this same instance. All fine for an internal PoC. → cache the metadata (CacheService); + turn SSRF back on for external targets. +- **Credential selection is fixed to the acting SA's internal credential.** No per-AS binding + yet (in-code TODO). +- **Security items deferred as in-code TODOs:** (a) confused-deputy guard — check that the + discovered `resource` equals the target we meant to call; (b) select the credential by the + discovered AS issuer; (c) external-AS SSRF/issuer-allowlist. A fourth is now on the list: the + `disabled` check on the machine path (see [Where the tracks meet §1](#1-disabled-is-an-off-switch-for-the-human-path-only)). + OBO adds three more (see [Delegated tokens](#delegated--on-behalf-of-tokens--internal-slice-built-this-session)). +- **No node `typeVersion` bump** — the new auth option is a backward-compatible enum value. - **Inline / sub-agents have no SA** → the mint hook fails closed. Acceptable; noted. -- **Provisioning actor = instance owner** (the `AgentsService.create` chain carries no human `User`). -- **License seats: issuance is resolved, the reported count isn't.** SAs are exempt from the quota check at cookie issuance (`auth.service.ts:302–315`) and creation skips it (`service-accounts.service.ts:112–113`), so nothing is blocked today. Still open: if a real seat count is ever added it must filter `type = 'user'` — the note is already at `user.repository.ts:98–101`, and `countUsersByRole` already does it. -- **No data migration for the secret** — bcrypt is one-way and Track 2 is unshipped, so dev/demo rows are recreated rather than migrated. +- **Provisioning actor = the instance owner** (the `AgentsService.create` call chain has no + human `User` to use). +- **License seats: issuance is handled, the reported count isn't.** SAs are exempt from the + quota check when a cookie is issued (`auth.service.ts:302–315`), and creation skips it + (`service-accounts.service.ts:112–113`), so nothing is blocked today. Still open: if a real + seat count is ever added, it must filter `type = 'user'` — the note is at + `user.repository.ts:98–101`, and `countUsersByRole` already does it. +- **No data migration for the secret** — bcrypt is one-way and Track 2 is unshipped, so + dev/demo rows get recreated rather than migrated. --- ## Known rough edges (Track 1 follow-ups still open) -Open, low-severity, recorded nowhere else. +Open, low-severity, and recorded nowhere else. -- **`PATCH /me/settings` 403s in every SA session.** Correctly guarded - (`me.controller.ts:325`), but the frontend calls it unprompted from - `useCalloutHelpers.ts:118` and `app/stores/workflows.store.ts:319,362`. Visible effect: - dismissed callouts don't stay dismissed while impersonating. +- **`PATCH /me/settings` returns 403 in every SA session.** Correctly guarded + (`me.controller.ts:325`), but the frontend calls it unprompted from `useCalloutHelpers.ts:118` + and `app/stores/workflows.store.ts:319,362`. Visible effect: dismissed callouts don't stay + dismissed while impersonating. - **`PersonalizationModal.vue:564`** sends you to the homepage whenever you aren't already - there. The SA fix routed *around* it (`users.store.ts:418`) rather than fixing it, so it - still yanks a human who followed a deep link on first login. Pre-existing wart. -- **An SA can ask for a workflow review but can never be a reviewer** — - `user.repository.ts:187–205` filters to real users, partly because sorting downstream + there. The SA fix routed *around* it (`users.store.ts:418`) rather than fixing it, so it still + yanks a human who followed a deep link on first login. A pre-existing wart. +- **An SA can request a workflow review but can never *be* a reviewer** — + `user.repository.ts:187–205` filters to real users, partly because the sorting downstream assumes a real email address. Probably right; worth confirming as a product decision. -- **Attribution gap.** Events and `updatedAt` during impersonation record the *SA*; the human - shows up only in logs and the five `service-account-*` events - (`relay.event-map.ts:325–354`). Track 3's audit rows cover tokens, not what the SA then did. -- **Multi-tab.** Tab B keeps a stale user in memory while its requests already go out as the - SA. Fixes itself on the next navigation. +- **Attribution of what the SA then *does*.** The account/impersonation lifecycle is now audited + (see [Audit trail](#audit-trail)), and tokens carry subject + actor. But regular domain + events and `updatedAt` written *during* impersonation still record the SA, not the human — + so "who changed this workflow while impersonating" is still only answerable from logs, not + from the row itself. +- **Multi-tab.** Tab B keeps a stale user in memory while its requests already go out as the SA. + It corrects itself on the next navigation. --- ## Gaps we wanted to research but didn't reach -### Delegated / on-behalf-of tokens — and what now stands in the way +### Delegated / on-behalf-of tokens — internal slice built (this session) -Still the biggest open item, but "no delegation exists" is the wrong framing. +This is no longer the biggest open item. The **internal** case is built and green (GATE-OBO). A +human-triggered agent run now mints an RFC 8693 delegated token — `{sub: human, act: {sub: SA}}` +— instead of the autonomous `sub = SA`, and the MCP-trigger boundary resolves both identities. -**It already exists.** Impersonation is real RFC 8693 delegation with a real `act` claim -(`auth.service.ts:32–48`), checked on every request (`:497`). The gap is narrower: delegation -works for **human→SA in the browser** and not on the **machine path**, where -`generateAccessTokenOnly` → `generateTokenPair` (`oauth-token.service.ts:53,69`) emits **no -`act` at all**. +**It always partly existed.** Impersonation is real RFC 8693 delegation, with a real `act` claim +(`auth.service.ts:32–48`) checked on every request (`:497`) — but only for **human→SA in the +browser**. The machine path (`generateAccessTokenOnly` → `generateTokenPair`, +`oauth-token.service.ts:53,69`) emitted **no `act`** at all. This slice adds `act` to the machine +path, in the **mirror** direction — impersonation is `sub=SA, act=human`; agent-OBO is +`sub=human, act=SA`. -**The obstacle: "who is acting" already has two opposite answers in the same vocabulary.** +**"Who is acting" already had two opposite answers. We picked a third, and said which.** | Path | Resolves to | Meaning | |---|---|---| | `scoped-jwt.strategy.ts:63`, `mcp-api-key.service.ts:76` | `actor ?? subject` — the **actor wins** | a service acts *for* a user, using its own authority | -| Impersonation (`auth.service.ts:37–43`) | the **subject** acts; `act` is ignored for authorization | a human drops their own authority and takes the SA's | +| Impersonation (`auth.service.ts:37–43`) | the **subject** acts; `act` is ignored for authz | a human sets aside their own authority and takes the SA's | +| **Agent-OBO (new)** | the **subject** (the human) authorizes; `act` = SA is kept for attribution | the agent acts *as* the human who triggered it, carrying its own identity alongside | -Both are deliberate. So an on-behalf-of token is a **third** choice, not an extension of -either — it has to pick one and say which. +**We avoided the concrete trap.** The `auth.service.ts` doc comment already warned that +`req.tokenGrant` readers do `actor ?? subject` — so filling in `actor` there would silently +**swap who is in charge**, agent for human, with no error anywhere. We treated that as a design +input, not something to trip over later: **the OBO path does not touch `req.tokenGrant`.** It +lives in the OAuth verify path (`verifyOAuthAccessToken`), which reads the subject from `sub`, +puts the actor in the *separate* `UserWithContext.actor` field (from `act.sub`), and +**authorizes against the subject** (`resource.authorize(user)`, where `user` is the human). +GATE-OBO pins this down: with the same SA and the same resource, a delegated-as-Alice token is +**accepted** (Alice has execute), and an autonomous-as-SA token is **denied** — so authorization +follows the human, not the agent. -**The concrete trap.** Don't route it through `req.tokenGrant`: both -`scoped-jwt.strategy.ts:87` and `mcp-api-key.service.ts:76` read -`tokenGrant.actor ?? tokenGrant.subject`, so filling in `actor` with the triggering human -would **silently swap who is in charge** — agent to human, with no error anywhere. The warning -is already in the `auth.service.ts` doc comment; it needs to be a design input, not something -found afterwards. +**How it's shaped (RFC 8693-correct), in two steps:** + +1. The SA mints its **own** access token via `client_credentials` (reusing the existing path). +2. A **token-exchange grant** (`urn:ietf:params:oauth:grant-type:token-exchange`, a sibling + middleware to `client_credentials` on the same `/oauth/token`) takes a `subject_token` (a + short-lived, `purpose`-tagged assertion the AS self-mints for the internal human) and an + `actor_token` (the SA token from step 1), and returns `{sub: human, act: {sub: SA}}`. + +The `act` claim comes from the **`actor_token`, not from client authentication**. (A first cut +derived it from `client_id`, which merged the OAuth client with the RFC actor and limited actors +to SAs; the fix means **any principal holding an access token can be an actor**.) The human is +threaded in via `IWorkflowExecuteAdditionalData.actingOnBehalfOfUserId`, set only on the +interactive/chat path where a human is actually present; the autonomous/published path stays +autonomous. + +**What "internal" buys, and what "external" still needs.** Internal works because n8n *is* the +human's authority — it can self-mint the `subject_token`. External delegation (a human whose +identity comes from an outside IdP) needs the **real inbound token threaded** from the request +down to the mint, to use as the `subject_token`, validated by the token-exchange module's +existing JWKS/identity-resolution half. Today we thread only a **userId**, not a live token. + +**Deferred (in-code TODOs):** `may_act` (is this actor allowed to act for this subject, and may +this client present this actor_token?); decoupling client-auth from the actor so a **non-SA** +actor works in this same slice (today the caller still authenticates as the SA's client); +external-AS `subject_token` threading. Minor: the two-step refactor dropped `userId`/`targetUrl` +from the mint-*failure* warn log (the audit event still carries `sub`/`aud`). ### Other gaps -- **`AgentsCredentialProvider` attribution** — traceability of third-party credential use to the SA. Dropped when decision 1 was made (the provider is genuinely untouched: it does not appear in `git diff --name-only master..HEAD`); worth revisiting as a separate, non-scoping mechanism (e.g. attribution metadata, not access-scoping). -- **Second call site** (`delegate-sub-agent-tool.ts`) for an agent-to-agent demo beat — not done. -- **External OAuth2 AS support** — designed-for (the discovery seam + credential↔AS binding), not built. +- **`AgentsCredentialProvider` attribution** — tracing third-party credential use back to the + SA. Dropped when decision 1 was made (the provider is genuinely untouched — it doesn't appear + in `git diff --name-only master..HEAD`). Worth revisiting as a *separate*, non-scoping + mechanism (attribution metadata, not access control). +- **Second call site** (`delegate-sub-agent-tool.ts`) for an agent-to-agent demo beat — not + done. +- **External OAuth2 AS support** — designed-for (the discovery seam + credential↔AS binding), + not built. - **Cross-instance / cross-org** — out of scope, as planned. --- ## Lessons -1. **A "free" type-compatible seam can still be semantically wrong.** The credential-provider `user` param accepted an SA, but SA project-scoping inverts the intent. Separate *identity that authorizes* from *identity a token represents* — they're different axes. -2. **The protocol loop was already half-built.** n8n *serves* RFC 9728 + 8414 on the resource side; only the client was hardcoding. Completing it was cheap and let us honestly claim "pure OAuth2 transport/discovery" for the internal case. -3. **Tests can pass through a fallback and hide a broken primary path.** The real-HTTP proof was green via the hardcoded fallback because the test server's `webhookBaseUrl` port didn't match — discovery was silently `ECONNREFUSED`-ing. Fix: assert the *observable* (the discovered `token_endpoint` value), not just the end result. -4. **The "real boundary" proof has a cost.** Choosing a real HTTP `client_credentials` round-trip over an in-process mint is what forced the reversible-secret change. Worth it for the demo, but the coupling was not obvious up front. -5. **A service account is a user-shaped hole — every implicit "is this a real person?" test is a latent bug, front end and back.** `password IS NULL`, `personalizationAnswers == null`, `settings == null`, `mfaEnabled === false` are all used as shorthand for "half-set-up human", and every one of them broke. **Corollary: fix the gate, not the stored data.** Backfilling `personalizationAnswers` at creation was rejected because it stores a claim the account can never make, is read as real downstream (`templates.store.ts:170`), ties SA creation to a versioned survey schema, and would only have helped SAs created *after* the fix. The guard went on the gate instead (`users.store.ts:418`). Same shape as lesson 1 — a seam that type-checks but means the wrong thing — except in data rather than an API. +1. **A "free" seam that type-checks can still be semantically wrong.** The credential-provider + `user` param accepted an SA, but SA project-scoping inverts what the param is *for*. Keep + "the identity that authorizes" and "the identity a token represents" as separate axes. +2. **The protocol loop was already half-built.** n8n *serves* RFC 9728 + 8414 on the resource + side; only the client was hardcoding. Finishing the loop was cheap, and it let us honestly + claim "pure OAuth2 transport/discovery" for the internal case. +3. **A test can pass through a fallback and hide a broken main path.** The real-HTTP proof was + green via the hardcoded fallback, because the test server's `webhookBaseUrl` port didn't + match and discovery was silently failing with `ECONNREFUSED`. Fix: assert the *observable* + (the discovered `token_endpoint` value), not just the end result. +4. **The "real boundary" proof has a cost.** Choosing a real HTTP `client_credentials` + round-trip over an in-process mint is exactly what forced the reversible-secret change. Worth + it for the demo, but the coupling wasn't obvious up front. +5. **A service account is a user-shaped hole — every implicit "is this a real person?" check is + a latent bug, front-end and back.** `password IS NULL`, `personalizationAnswers == null`, + `settings == null`, `mfaEnabled === false` are all used as shorthand for "half-set-up human", + and every one of them broke. **Corollary: fix the gate, not the stored data.** Backfilling + `personalizationAnswers` at creation was rejected because it stores a claim the account can + never make, is read as real downstream (`templates.store.ts:170`), ties SA creation to a + versioned survey schema, and would only have helped SAs created *after* the fix. The guard + went on the gate instead (`users.store.ts:418`). Same shape as lesson 1 — a seam that + type-checks but means the wrong thing — only in data rather than an API. --- ## Where it stands -All green: full monorepo build (69/69), 280+ unit/integration tests, 0 lint errors. Proof -gates GATE-1/2/3/4/6 pass. Runbook: `packages/cli/src/modules/oauth-server/SERVICE_ACCOUNT_PROOF.md`. -Everything is on `afitzek/track-2-implementation` (Track 3 + discovery uncommitted). \ No newline at end of file +All green. The proof gates GATE-1/2/3/4/6 (autonomous self-auth) **and GATE-OBO** (delegated +`{sub: human, act: SA}` with authorize-as-subject) pass. The OBO slice is RFC 8693-correct +(`act` from the `actor_token`, two-step mint), and the audit trail records subject + actor +across the token lifecycle plus the account/impersonation lifecycle. The oauth-server / OBO / +audit suites are green (255 tests at last run), and cli typecheck + lint are clean. The last +full monorepo build was 69/69 (before OBO; OBO is cli-only plus one rebuilt `n8n-workflow` +interface field, and it composes clean). + +The audit events are also browsable in-instance via the opt-in DB sink (see +[Seeing the trail](#seeing-the-trail-the-db-sink-poc)) — a new `audit_log_event` table +(migration `1785900000001`), a `GET /rest/eventbus/audit-log-events` endpoint, and a settings +page; it compiles clean and its tests pass (14). + +Runbook: `packages/cli/src/modules/oauth-server/SERVICE_ACCOUNT_PROOF.md`. Everything is on +`afitzek/track-2-implementation` (Track 3 + discovery + OBO + audit + the audit-log DB sink +uncommitted). diff --git a/docs/generated/postgres-schema/README.md b/docs/generated/postgres-schema/README.md index 5e0e8086b09..7cd15364df4 100644 --- a/docs/generated/postgres-schema/README.md +++ b/docs/generated/postgres-schema/README.md @@ -35,6 +35,7 @@ Auto-generated from the PostgreSQL migrations in @n8n/db. Do not edit by hand. | [public.agents_threads](public.agents_threads.md) | 6 | | BASE TABLE | | [public.ai_builder_temporary_workflow](public.ai_builder_temporary_workflow.md) | 4 | | BASE TABLE | | [public.annotation_tag_entity](public.annotation_tag_entity.md) | 4 | | BASE TABLE | +| [public.audit_log_event](public.audit_log_event.md) | 7 | | BASE TABLE | | [public.auth_identity](public.auth_identity.md) | 5 | | BASE TABLE | | [public.auth_provider_sync_history](public.auth_provider_sync_history.md) | 11 | | BASE TABLE | | [public.binary_data](public.binary_data.md) | 9 | | BASE TABLE | @@ -634,6 +635,15 @@ erDiagram varchar_24_ name timestamp_3__with_time_zone updatedAt } +"public.audit_log_event" { + timestamp_3__with_time_zone createdAt + varchar_255_ eventName + varchar_36_ id + varchar_255_ message + json payload + timestamp_3__with_time_zone ts + timestamp_3__with_time_zone updatedAt +} "public.auth_identity" { timestamp_3__with_time_zone createdAt varchar_255_ providerId diff --git a/docs/generated/postgres-schema/public.audit_log_event.md b/docs/generated/postgres-schema/public.audit_log_event.md new file mode 100644 index 00000000000..19e3c2c63c3 --- /dev/null +++ b/docs/generated/postgres-schema/public.audit_log_event.md @@ -0,0 +1,53 @@ +# public.audit_log_event + +## Columns + +| Name | Type | Default | Nullable | Children | Parents | Comment | +| ---- | ---- | ------- | -------- | -------- | ------- | ------- | +| createdAt | timestamp(3) with time zone | CURRENT_TIMESTAMP(3) | false | | | | +| eventName | varchar(255) | | false | | | Dotted event name, e.g. n8n.audit.* | +| id | varchar(36) | | false | | | Originating EventMessage id | +| message | varchar(255) | | false | | | | +| payload | json | | true | | | | +| ts | timestamp(3) with time zone | | false | | | The event's own timestamp | +| updatedAt | timestamp(3) with time zone | CURRENT_TIMESTAMP(3) | false | | | | + +## Constraints + +| Name | Type | Definition | +| ---- | ---- | ---------- | +| PK_c37ba8970192420a944eaef393d | PRIMARY KEY | PRIMARY KEY (id) | +| audit_log_event_createdAt_not_null | n | NOT NULL "createdAt" | +| audit_log_event_eventName_not_null | n | NOT NULL "eventName" | +| audit_log_event_id_not_null | n | NOT NULL id | +| audit_log_event_message_not_null | n | NOT NULL message | +| audit_log_event_ts_not_null | n | NOT NULL ts | +| audit_log_event_updatedAt_not_null | n | NOT NULL "updatedAt" | + +## Indexes + +| Name | Definition | +| ---- | ---------- | +| IDX_ea9ed51f539d2ca4ba8e4fe342 | CREATE INDEX "IDX_ea9ed51f539d2ca4ba8e4fe342" ON public.audit_log_event USING btree ("eventName") | +| PK_c37ba8970192420a944eaef393d | CREATE UNIQUE INDEX "PK_c37ba8970192420a944eaef393d" ON public.audit_log_event USING btree (id) | + +## Relations + +```mermaid +erDiagram + + +"public.audit_log_event" { + timestamp_3__with_time_zone createdAt + varchar_255_ eventName + varchar_36_ id + varchar_255_ message + json payload + timestamp_3__with_time_zone ts + timestamp_3__with_time_zone updatedAt +} +``` + +--- + +> Generated by [tbls](https://github.com/k1LoW/tbls) diff --git a/docs/generated/sqlite-schema/README.md b/docs/generated/sqlite-schema/README.md index b72a83694da..c284fc65d15 100644 --- a/docs/generated/sqlite-schema/README.md +++ b/docs/generated/sqlite-schema/README.md @@ -35,6 +35,7 @@ Auto-generated from the SQLite migrations in @n8n/db. Do not edit by hand. | [agents_threads](agents_threads.md) | 6 | | table | | [ai_builder_temporary_workflow](ai_builder_temporary_workflow.md) | 4 | | table | | [annotation_tag_entity](annotation_tag_entity.md) | 4 | | table | +| [audit_log_event](audit_log_event.md) | 7 | | table | | [auth_identity](auth_identity.md) | 5 | | table | | [auth_provider_sync_history](auth_provider_sync_history.md) | 11 | | table | | [binary_data](binary_data.md) | 9 | | table | @@ -621,6 +622,15 @@ erDiagram varchar_24_ name datetime_3_ updatedAt } +"audit_log_event" { + datetime_3_ createdAt + varchar_255_ eventName + varchar_36_ id PK + varchar_255_ message + TEXT payload + datetime_3_ ts + datetime_3_ updatedAt +} "auth_identity" { timestamp createdAt VARCHAR_64_ providerId PK diff --git a/docs/generated/sqlite-schema/audit_log_event.md b/docs/generated/sqlite-schema/audit_log_event.md new file mode 100644 index 00000000000..aaad6ed82c6 --- /dev/null +++ b/docs/generated/sqlite-schema/audit_log_event.md @@ -0,0 +1,59 @@ +# audit_log_event + +## Description + +
+Table Definition + +```sql +CREATE TABLE "audit_log_event" ("id" varchar(36) PRIMARY KEY NOT NULL, "eventName" varchar(255) NOT NULL, "message" varchar(255) NOT NULL, "ts" datetime(3) NOT NULL, "payload" text, "createdAt" datetime(3) NOT NULL DEFAULT (STRFTIME('%Y-%m-%d %H:%M:%f', 'NOW')), "updatedAt" datetime(3) NOT NULL DEFAULT (STRFTIME('%Y-%m-%d %H:%M:%f', 'NOW'))) +``` + +
+ +## Columns + +| Name | Type | Default | Nullable | Children | Parents | Comment | +| ---- | ---- | ------- | -------- | -------- | ------- | ------- | +| createdAt | datetime(3) | STRFTIME('%Y-%m-%d %H:%M:%f', 'NOW') | false | | | | +| eventName | varchar(255) | | false | | | | +| id | varchar(36) | | false | | | | +| message | varchar(255) | | false | | | | +| payload | TEXT | | true | | | | +| ts | datetime(3) | | false | | | | +| updatedAt | datetime(3) | STRFTIME('%Y-%m-%d %H:%M:%f', 'NOW') | false | | | | + +## Constraints + +| Name | Type | Definition | +| ---- | ---- | ---------- | +| id | PRIMARY KEY | PRIMARY KEY (id) | +| sqlite_autoindex_audit_log_event_1 | PRIMARY KEY | PRIMARY KEY (id) | + +## Indexes + +| Name | Definition | +| ---- | ---------- | +| IDX_ea9ed51f539d2ca4ba8e4fe342 | CREATE INDEX "IDX_ea9ed51f539d2ca4ba8e4fe342" ON "audit_log_event" ("eventName") | +| sqlite_autoindex_audit_log_event_1 | PRIMARY KEY (id) | + +## Relations + +```mermaid +erDiagram + + +"audit_log_event" { + datetime_3_ createdAt + varchar_255_ eventName + varchar_36_ id PK + varchar_255_ message + TEXT payload + datetime_3_ ts + datetime_3_ updatedAt +} +``` + +--- + +> Generated by [tbls](https://github.com/k1LoW/tbls) diff --git a/packages/@n8n/api-types/src/dto/index.ts b/packages/@n8n/api-types/src/dto/index.ts index c2836a830ce..68655bee0aa 100644 --- a/packages/@n8n/api-types/src/dto/index.ts +++ b/packages/@n8n/api-types/src/dto/index.ts @@ -166,6 +166,7 @@ export { } from './log-streaming/create-destination.dto'; export { TestDestinationQueryDto } from './log-streaming/test-destination-query.dto'; export { DeleteDestinationQueryDto } from './log-streaming/delete-destination-query.dto'; +export { ListAuditLogEventsQueryDto } from './log-streaming/list-audit-log-events.dto'; export { PublicCreateDestinationDto, PublicDestinationResponseDto, diff --git a/packages/@n8n/api-types/src/dto/log-streaming/list-audit-log-events.dto.ts b/packages/@n8n/api-types/src/dto/log-streaming/list-audit-log-events.dto.ts new file mode 100644 index 00000000000..9329316b1e0 --- /dev/null +++ b/packages/@n8n/api-types/src/dto/log-streaming/list-audit-log-events.dto.ts @@ -0,0 +1,10 @@ +import { z } from 'zod'; + +import { paginationSchema } from '../pagination/pagination.dto'; +import { Z } from '../../zod-class'; + +export class ListAuditLogEventsQueryDto extends Z.class({ + ...paginationSchema, + // Event-name prefix filter, e.g. "n8n.audit". + prefix: z.string().trim().min(1).max(128).optional(), +}) {} diff --git a/packages/@n8n/db/src/migrations/common/1785900000001-CreateAuditLogEventTable.ts b/packages/@n8n/db/src/migrations/common/1785900000001-CreateAuditLogEventTable.ts new file mode 100644 index 00000000000..b94481d496f --- /dev/null +++ b/packages/@n8n/db/src/migrations/common/1785900000001-CreateAuditLogEventTable.ts @@ -0,0 +1,21 @@ +import type { MigrationContext, ReversibleMigration } from '../migration-types'; + +export class CreateAuditLogEventTable1785900000001 implements ReversibleMigration { + async up(ctx: MigrationContext) { + const { createTable, column } = ctx.schemaBuilder; + + await createTable('audit_log_event') + .withColumns( + column('id').varchar(36).primary.comment('Originating EventMessage id'), + column('eventName').varchar(255).notNull.comment('Dotted event name, e.g. n8n.audit.*'), + column('message').varchar(255).notNull, + column('ts').timestampTimezone().notNull.comment("The event's own timestamp"), + column('payload').json, + ) + .withIndexOn(['eventName']).withTimestamps; + } + + async down(ctx: MigrationContext) { + await ctx.schemaBuilder.dropTable('audit_log_event'); + } +} diff --git a/packages/@n8n/db/src/migrations/postgresdb/index.ts b/packages/@n8n/db/src/migrations/postgresdb/index.ts index 182afff649a..d8e1fd38afd 100644 --- a/packages/@n8n/db/src/migrations/postgresdb/index.ts +++ b/packages/@n8n/db/src/migrations/postgresdb/index.ts @@ -240,6 +240,7 @@ import { AddSetupCompletedAtToAgents1785500832626 } from '../common/178550083262 import { AddServiceAccountCredentials1785500832627 } from '../common/1785500832627-AddServiceAccountCredentials'; import { AddTypeToUser1785763124126 } from '../common/1785763124126-AddTypeToUser'; import { AddServiceAccountUserIdToAgents1785900000000 } from '../common/1785900000000-AddServiceAccountUserIdToAgents'; +import { CreateAuditLogEventTable1785900000001 } from '../common/1785900000001-CreateAuditLogEventTable'; import type { Migration } from '../migration-types'; export const postgresMigrations: Migration[] = [ @@ -485,4 +486,5 @@ export const postgresMigrations: Migration[] = [ AddServiceAccountCredentials1785500832627, AddTypeToUser1785763124126, AddServiceAccountUserIdToAgents1785900000000, + CreateAuditLogEventTable1785900000001, ]; diff --git a/packages/@n8n/db/src/migrations/sqlite/index.ts b/packages/@n8n/db/src/migrations/sqlite/index.ts index 68715663a51..053d914822f 100644 --- a/packages/@n8n/db/src/migrations/sqlite/index.ts +++ b/packages/@n8n/db/src/migrations/sqlite/index.ts @@ -232,6 +232,7 @@ import { CreateAgentChatAttachmentsTable1785255306000 } from '../common/17852553 import { AddServiceAccountCredentials1785500832627 } from '../common/1785500832627-AddServiceAccountCredentials'; import { AddTypeToUser1785763124126 } from '../common/1785763124126-AddTypeToUser'; import { AddServiceAccountUserIdToAgents1785900000000 } from './1785900000000-AddServiceAccountUserIdToAgents'; +import { CreateAuditLogEventTable1785900000001 } from '../common/1785900000001-CreateAuditLogEventTable'; const sqliteMigrations: Migration[] = [ InitialMigration1588102412422, @@ -467,6 +468,7 @@ const sqliteMigrations: Migration[] = [ AddServiceAccountCredentials1785500832627, AddTypeToUser1785763124126, AddServiceAccountUserIdToAgents1785900000000, + CreateAuditLogEventTable1785900000001, ]; export { sqliteMigrations }; diff --git a/packages/cli/build-fe.log b/packages/cli/build-fe.log new file mode 100644 index 00000000000..e69de29bb2d diff --git a/packages/cli/src/eventbus/event-message-classes/index.ts b/packages/cli/src/eventbus/event-message-classes/index.ts index 46d543315e9..a141269d959 100644 --- a/packages/cli/src/eventbus/event-message-classes/index.ts +++ b/packages/cli/src/eventbus/event-message-classes/index.ts @@ -126,6 +126,12 @@ export const eventNamesAudit = [ 'n8n.audit.token-exchange.role-updated', 'n8n.audit.service-account.token.minted', 'n8n.audit.service-account.token.verified', + 'n8n.audit.service-account.token.exchanged', + 'n8n.audit.service-account.created', + 'n8n.audit.service-account.role-changed', + 'n8n.audit.service-account.deleted', + 'n8n.audit.service-account.impersonation.started', + 'n8n.audit.service-account.impersonation.ended', 'n8n.audit.role-mapping.roles-resolved', 'n8n.audit.role-mapping.rule.created', 'n8n.audit.role-mapping.rule.updated', diff --git a/packages/cli/src/events/__tests__/log-streaming-event-relay.test.ts b/packages/cli/src/events/__tests__/log-streaming-event-relay.test.ts index 2c40e35fee9..062a6ee4868 100644 --- a/packages/cli/src/events/__tests__/log-streaming-event-relay.test.ts +++ b/packages/cli/src/events/__tests__/log-streaming-event-relay.test.ts @@ -3151,4 +3151,129 @@ describe('LogStreamingEventRelay', () => { }); }); }); + + describe('service account events', () => { + it('should log on `service-account-token-minted` event', () => { + const event: RelayEventMap['service-account-token-minted'] = { + sub: 'human-1', + act: 'sa-1', + clientId: 'client-1', + aud: 'https://n8n.example.com/mcp', + outcome: 'success', + }; + + eventService.emit('service-account-token-minted', event); + + expect(eventBus.sendAuditEvent).toHaveBeenCalledWith({ + eventName: 'n8n.audit.service-account.token.minted', + payload: event, + }); + }); + + it('should log on `service-account-token-verified` event', () => { + const event: RelayEventMap['service-account-token-verified'] = { + sub: 'human-1', + act: 'sa-1', + aud: 'https://n8n.example.com/mcp', + outcome: 'success', + }; + + eventService.emit('service-account-token-verified', event); + + expect(eventBus.sendAuditEvent).toHaveBeenCalledWith({ + eventName: 'n8n.audit.service-account.token.verified', + payload: event, + }); + }); + + it('should log on `service-account-token-exchanged` event', () => { + const event: RelayEventMap['service-account-token-exchanged'] = { + sub: 'human-1', + act: 'sa-1', + clientId: 'client-1', + aud: 'https://n8n.example.com/mcp', + outcome: 'success', + }; + + eventService.emit('service-account-token-exchanged', event); + + expect(eventBus.sendAuditEvent).toHaveBeenCalledWith({ + eventName: 'n8n.audit.service-account.token.exchanged', + payload: event, + }); + }); + + it('should log on `service-account-created` event', () => { + const event: RelayEventMap['service-account-created'] = { + userId: 'human-1', + serviceAccountId: 'sa-1', + serviceAccountRole: 'global:member', + }; + + eventService.emit('service-account-created', event); + + expect(eventBus.sendAuditEvent).toHaveBeenCalledWith({ + eventName: 'n8n.audit.service-account.created', + payload: event, + }); + }); + + it('should log on `service-account-role-changed` event', () => { + const event: RelayEventMap['service-account-role-changed'] = { + userId: 'human-1', + serviceAccountId: 'sa-1', + serviceAccountNewRole: 'global:admin', + }; + + eventService.emit('service-account-role-changed', event); + + expect(eventBus.sendAuditEvent).toHaveBeenCalledWith({ + eventName: 'n8n.audit.service-account.role-changed', + payload: event, + }); + }); + + it('should log on `service-account-deleted` event', () => { + const event: RelayEventMap['service-account-deleted'] = { + userId: 'human-1', + serviceAccountId: 'sa-1', + migrationStrategy: 'delete_data', + }; + + eventService.emit('service-account-deleted', event); + + expect(eventBus.sendAuditEvent).toHaveBeenCalledWith({ + eventName: 'n8n.audit.service-account.deleted', + payload: event, + }); + }); + + it('should log on `service-account-impersonation-started` event', () => { + const event: RelayEventMap['service-account-impersonation-started'] = { + userId: 'human-1', + serviceAccountId: 'sa-1', + }; + + eventService.emit('service-account-impersonation-started', event); + + expect(eventBus.sendAuditEvent).toHaveBeenCalledWith({ + eventName: 'n8n.audit.service-account.impersonation.started', + payload: event, + }); + }); + + it('should log on `service-account-impersonation-ended` event', () => { + const event: RelayEventMap['service-account-impersonation-ended'] = { + userId: 'human-1', + serviceAccountId: 'sa-1', + }; + + eventService.emit('service-account-impersonation-ended', event); + + expect(eventBus.sendAuditEvent).toHaveBeenCalledWith({ + eventName: 'n8n.audit.service-account.impersonation.ended', + payload: event, + }); + }); + }); }); diff --git a/packages/cli/src/events/maps/relay.event-map.ts b/packages/cli/src/events/maps/relay.event-map.ts index db1d40d432d..f8df3620b4d 100644 --- a/packages/cli/src/events/maps/relay.event-map.ts +++ b/packages/cli/src/events/maps/relay.event-map.ts @@ -1020,6 +1020,10 @@ export type RelayEventMap = { // account mints a token via client_credentials. 'service-account-token-minted': { sub: string; + // RFC 8693 actor: set only for a delegated (on-behalf-of) mint, where `sub` + // is the human subject and `act` is the acting service account. Absent for + // an autonomous mint (`sub` is the service account itself). + act?: string; clientId: string; aud: string; outcome: 'success' | 'failure'; @@ -1029,6 +1033,20 @@ export type RelayEventMap = { // service-account token is verified against a protected resource. 'service-account-token-verified': { sub: string | null; + // RFC 8693 actor resolved from the token's `act.sub`; null/absent when the + // token carries no delegation. + act?: string | null; + aud: string; + outcome: 'success' | 'failure'; + }; + + // Emitted by `OAuthServerService.exchangeToken` when the RFC 8693 + // token-exchange grant mints a delegated token at the authorization server. + // `sub` is the human subject, `act` the acting service account. + 'service-account-token-exchanged': { + sub: string | null; + act: string | null; + clientId: string | null; aud: string; outcome: 'success' | 'failure'; }; diff --git a/packages/cli/src/events/relays/log-streaming.event-relay.ts b/packages/cli/src/events/relays/log-streaming.event-relay.ts index cfecf255543..1155ca48867 100644 --- a/packages/cli/src/events/relays/log-streaming.event-relay.ts +++ b/packages/cli/src/events/relays/log-streaming.event-relay.ts @@ -137,6 +137,14 @@ export class LogStreamingEventRelay extends EventRelay { 'token-exchange-role-updated': (event) => this.tokenExchangeRoleUpdated(event), 'service-account-token-minted': (event) => this.serviceAccountTokenMinted(event), 'service-account-token-verified': (event) => this.serviceAccountTokenVerified(event), + 'service-account-token-exchanged': (event) => this.serviceAccountTokenExchanged(event), + 'service-account-created': (event) => this.serviceAccountCreated(event), + 'service-account-role-changed': (event) => this.serviceAccountRoleChanged(event), + 'service-account-deleted': (event) => this.serviceAccountDeleted(event), + 'service-account-impersonation-started': (event) => + this.serviceAccountImpersonationStarted(event), + 'service-account-impersonation-ended': (event) => + this.serviceAccountImpersonationEnded(event), 'embed-login': (event) => this.embedLogin(event), 'embed-login-failed': (event) => this.embedLoginFailed(event), 'expression-mapping-roles-resolved': (event) => this.expressionMappingRolesResolved(event), @@ -1211,6 +1219,55 @@ export class LogStreamingEventRelay extends EventRelay { }); } + private serviceAccountTokenExchanged(event: RelayEventMap['service-account-token-exchanged']) { + void this.eventBus.sendAuditEvent({ + eventName: 'n8n.audit.service-account.token.exchanged', + payload: event, + }); + } + + // `userId` on these lifecycle events is the human actor (never the SA), so the + // payload carries a plain id — no `@Redactable` (that decorator redacts a + // `user`/`UserLike` object, which these events don't carry). + private serviceAccountCreated(event: RelayEventMap['service-account-created']) { + void this.eventBus.sendAuditEvent({ + eventName: 'n8n.audit.service-account.created', + payload: event, + }); + } + + private serviceAccountRoleChanged(event: RelayEventMap['service-account-role-changed']) { + void this.eventBus.sendAuditEvent({ + eventName: 'n8n.audit.service-account.role-changed', + payload: event, + }); + } + + private serviceAccountDeleted(event: RelayEventMap['service-account-deleted']) { + void this.eventBus.sendAuditEvent({ + eventName: 'n8n.audit.service-account.deleted', + payload: event, + }); + } + + private serviceAccountImpersonationStarted( + event: RelayEventMap['service-account-impersonation-started'], + ) { + void this.eventBus.sendAuditEvent({ + eventName: 'n8n.audit.service-account.impersonation.started', + payload: event, + }); + } + + private serviceAccountImpersonationEnded( + event: RelayEventMap['service-account-impersonation-ended'], + ) { + void this.eventBus.sendAuditEvent({ + eventName: 'n8n.audit.service-account.impersonation.ended', + payload: event, + }); + } + private embedLogin(event: RelayEventMap['embed-login']) { void this.eventBus.sendAuditEvent({ eventName: 'n8n.audit.token-exchange.embed-login', diff --git a/packages/cli/src/modules/agents/__tests__/agent-runtime-cache.service.test.ts b/packages/cli/src/modules/agents/__tests__/agent-runtime-cache.service.test.ts index 95ed0b87c98..1e33ca17dff 100644 --- a/packages/cli/src/modules/agents/__tests__/agent-runtime-cache.service.test.ts +++ b/packages/cli/src/modules/agents/__tests__/agent-runtime-cache.service.test.ts @@ -117,6 +117,7 @@ describe('AgentRuntimeCacheService', () => { undefined, undefined, undefined, + undefined, ); }); @@ -133,6 +134,26 @@ describe('AgentRuntimeCacheService', () => { expect(reconstructionService.reconstructFromAgentEntity.mock.calls[0]?.[6]).toBe('sa-user-1'); }); + it('threads the triggering human as the on-behalf-of identity, only when a user is present', async () => { + const { service, agentRepository, reconstructionService } = makeService(); + const agent = makeAgent(); + const user = mock({ id: 'human-1' }); + + agentRepository.findByIdAndProjectId.mockResolvedValue(agent); + reconstructionService.reconstructFromAgentEntity.mockResolvedValue(makeRuntime()); + actingIdentityResolver.mockResolvedValue('sa-user-1'); + + // Human-triggered (interactive) run: the human is threaded as the OBO identity + // alongside the acting SA, so downstream minting delegates. + await service.getRuntime({ agentId, projectId, user }); + expect(reconstructionService.reconstructFromAgentEntity.mock.calls[0]?.[6]).toBe('sa-user-1'); + expect(reconstructionService.reconstructFromAgentEntity.mock.calls[0]?.[7]).toBe('human-1'); + + // Integration run (no user, distinct cache key): no OBO identity → autonomous minting. + await service.getRuntime({ agentId, projectId, integrationType: 'slack' }); + expect(reconstructionService.reconstructFromAgentEntity.mock.calls[1]?.[7]).toBeUndefined(); + }); + it('keeps draft runtimes separate by integration type', async () => { const { service, agentRepository, reconstructionService } = makeService(); const agent = makeAgent(); @@ -163,6 +184,7 @@ describe('AgentRuntimeCacheService', () => { undefined, undefined, undefined, + undefined, ); }); @@ -196,6 +218,7 @@ describe('AgentRuntimeCacheService', () => { userA, undefined, undefined, + 'user-a', ); expect(reconstructionService.reconstructFromAgentEntity).toHaveBeenNthCalledWith( 2, @@ -206,6 +229,7 @@ describe('AgentRuntimeCacheService', () => { userB, undefined, undefined, + 'user-b', ); }); @@ -336,6 +360,7 @@ describe('AgentRuntimeCacheService', () => { undefined, undefined, undefined, + undefined, ); }); diff --git a/packages/cli/src/modules/agents/agent-runtime-cache.service.ts b/packages/cli/src/modules/agents/agent-runtime-cache.service.ts index 5871a9e1dba..6438ef01045 100644 --- a/packages/cli/src/modules/agents/agent-runtime-cache.service.ts +++ b/packages/cli/src/modules/agents/agent-runtime-cache.service.ts @@ -239,6 +239,12 @@ export class AgentRuntimeCacheService { const actingServiceAccountUserId = await Container.get(AgentsService).resolveActingServiceAccountUserId(agentEntity); + // When a human triggered this run (in-app chat/resume/task-now) the agent + // acts on their behalf: thread the human so outbound minting is delegated + // (sub = human, act = agent SA). Published/integration runs have no user + // and keep autonomous minting (sub = agent SA). + const actingOnBehalfOfUserId = user?.id; + const { agent: agentInstance, toolRegistry } = await this.agentRuntimeReconstructionService.reconstructFromAgentEntity( agentData, @@ -248,6 +254,7 @@ export class AgentRuntimeCacheService { user, undefined, actingServiceAccountUserId, + actingOnBehalfOfUserId, ); return { diff --git a/packages/cli/src/modules/agents/agent-runtime-reconstruction.service.ts b/packages/cli/src/modules/agents/agent-runtime-reconstruction.service.ts index 4333111929a..b53d3744267 100644 --- a/packages/cli/src/modules/agents/agent-runtime-reconstruction.service.ts +++ b/packages/cli/src/modules/agents/agent-runtime-reconstruction.service.ts @@ -185,6 +185,7 @@ export class AgentRuntimeReconstructionService { user?: User, instrumentation?: AgentRuntimeInstrumentation, actingServiceAccountUserId?: string, + actingOnBehalfOfUserId?: string, ): Promise<{ agent: RuntimeAgent; toolRegistry: ToolRegistry }> { let config = agentEntity.schema; if (!config) { @@ -232,6 +233,7 @@ export class AgentRuntimeReconstructionService { user, instrumentation, actingServiceAccountUserId, + actingOnBehalfOfUserId, }); } @@ -349,6 +351,7 @@ export class AgentRuntimeReconstructionService { user?: User; instrumentation?: AgentRuntimeInstrumentation; actingServiceAccountUserId?: string; + actingOnBehalfOfUserId?: string; }): Promise<{ agent: RuntimeAgent; toolRegistry: ToolRegistry }> { const { config, @@ -367,6 +370,7 @@ export class AgentRuntimeReconstructionService { user, instrumentation, actingServiceAccountUserId, + actingOnBehalfOfUserId, } = options; const toolExecutor = this.secureRuntime.createToolExecutor(toolCodeByName); @@ -374,6 +378,7 @@ export class AgentRuntimeReconstructionService { projectId, instrumentation, actingServiceAccountUserId, + actingOnBehalfOfUserId, ); const resolvedTools: BuiltTool[] = []; @@ -397,6 +402,7 @@ export class AgentRuntimeReconstructionService { oauthService: this.oauthService, internalOAuth2MintService: this.internalOAuth2MintService, actingServiceAccountUserId, + actingOnBehalfOfUserId, projectId, proxyFetch: aiMcpFetch, onConnectionFailed: (event) => { @@ -530,6 +536,7 @@ export class AgentRuntimeReconstructionService { projectId: string, instrumentation?: AgentRuntimeInstrumentation, actingServiceAccountUserId?: string, + actingOnBehalfOfUserId?: string, ): ToolResolver { const instrumentToolAdditionalData = instrumentation?.configureToolAdditionalData; return async (ref: AgentJsonToolConfig) => { @@ -551,6 +558,7 @@ export class AgentRuntimeReconstructionService { executor: this.ephemeralNodeExecutor, projectId, actingServiceAccountUserId, + actingOnBehalfOfUserId, instrumentToolAdditionalData, }); } diff --git a/packages/cli/src/modules/agents/json-config/__tests__/mcp-client-factory.test.ts b/packages/cli/src/modules/agents/json-config/__tests__/mcp-client-factory.test.ts index 0c892f5aeac..4cf464110b1 100644 --- a/packages/cli/src/modules/agents/json-config/__tests__/mcp-client-factory.test.ts +++ b/packages/cli/src/modules/agents/json-config/__tests__/mcp-client-factory.test.ts @@ -186,10 +186,31 @@ describe('buildMcpClientForServer — n8nInternalOAuth2', () => { expect(internalOAuth2MintService.mintForUser).toHaveBeenCalledWith( 'sa-user-1', 'https://example.test/mcp', + {}, + undefined, ); expect(headers.Authorization).toBe('Bearer minted-token'); }); + it('delegates the mint (passes the on-behalf-of human) on a human-triggered run', async () => { + const internalOAuth2MintService = mock(); + internalOAuth2MintService.mintForUser.mockResolvedValue('delegated-token'); + + const headers = await buildAndCaptureHeaders({ + internalOAuth2MintService, + actingServiceAccountUserId: 'sa-user-1', + actingOnBehalfOfUserId: 'human-1', + }); + + expect(internalOAuth2MintService.mintForUser).toHaveBeenCalledWith( + 'sa-user-1', + 'https://example.test/mcp', + {}, + 'human-1', + ); + expect(headers.Authorization).toBe('Bearer delegated-token'); + }); + it('fails closed (no auth header, no mint) when there is no acting identity', async () => { const internalOAuth2MintService = mock(); diff --git a/packages/cli/src/modules/agents/json-config/mcp-client-factory.ts b/packages/cli/src/modules/agents/json-config/mcp-client-factory.ts index 35970ea487e..2260f88472b 100644 --- a/packages/cli/src/modules/agents/json-config/mcp-client-factory.ts +++ b/packages/cli/src/modules/agents/json-config/mcp-client-factory.ts @@ -140,17 +140,21 @@ async function deriveAuthHeaders( credentialProvider: CredentialProvider, internalOAuth2MintService?: InternalOAuth2MintService, actingServiceAccountUserId?: string, + actingOnBehalfOfUserId?: string, ): Promise { if (server.authentication === 'n8nInternalOAuth2') { // Self-authenticate as the agent's service-account identity: no credential // fetch (the marker holds no secret) and the target URL is the token // audience. Fails closed — without an acting identity (or the mint service, // on build-time paths) there is no token, so the request goes out - // unauthenticated and the server rejects it. + // unauthenticated and the server rejects it. On a human-triggered run + // `actingOnBehalfOfUserId` delegates the mint (sub = human, act = SA). if (!actingServiceAccountUserId || !internalOAuth2MintService) return { headers: {} }; const token = await internalOAuth2MintService.mintForUser( actingServiceAccountUserId, server.url, + {}, + actingOnBehalfOfUserId, ); return { headers: { Authorization: `Bearer ${token}` } }; } @@ -185,6 +189,12 @@ export interface BuildMcpClientDeps { * and fail closed at the server). */ actingServiceAccountUserId?: string; + /** + * Human on whose behalf an interactive run acts. When set alongside + * `actingServiceAccountUserId`, the `n8nInternalOAuth2` mint is delegated + * (sub = human, act = SA) instead of autonomous. Absent on autonomous runs. + */ + actingOnBehalfOfUserId?: string; projectId: string; proxyFetch: CustomFetch; /** @@ -214,6 +224,7 @@ export async function buildMcpClientForServer( oauthService, internalOAuth2MintService, actingServiceAccountUserId, + actingOnBehalfOfUserId, projectId, proxyFetch, onConnectionFailed, @@ -226,6 +237,7 @@ export async function buildMcpClientForServer( credentialProvider, internalOAuth2MintService, actingServiceAccountUserId, + actingOnBehalfOfUserId, ); const allowedDomains = credentialData ? resolveAllowedDomains(credentialData) : undefined; diff --git a/packages/cli/src/modules/agents/tools/node-tool-factory.ts b/packages/cli/src/modules/agents/tools/node-tool-factory.ts index 19f150f132b..d7d11e02182 100644 --- a/packages/cli/src/modules/agents/tools/node-tool-factory.ts +++ b/packages/cli/src/modules/agents/tools/node-tool-factory.ts @@ -24,6 +24,12 @@ export interface NodeToolFactoryContext { * the agent's service account. Server-set only — never from node input. */ actingServiceAccountUserId?: string; + /** + * Human on whose behalf an interactive run acts. When set alongside + * `actingServiceAccountUserId`, the node's self-authentication mint is + * delegated (sub = human, act = SA). Server-set only; absent on autonomous runs. + */ + actingOnBehalfOfUserId?: string; /** Eval-only additionalData decoration — absent on every production path. */ instrumentToolAdditionalData?: InstrumentToolAdditionalData; } @@ -170,6 +176,7 @@ export async function resolveNodeTool( inputData: [{ json: input as IDataObject }], projectId: ctx.projectId, actingServiceAccountUserId: ctx.actingServiceAccountUserId, + actingOnBehalfOfUserId: ctx.actingOnBehalfOfUserId, ...instrumentedExecution, }); // Throw on the executor's structured error so the agent runtime diff --git a/packages/cli/src/modules/log-streaming.ee/__tests__/log-streaming.controller.test.ts b/packages/cli/src/modules/log-streaming.ee/__tests__/log-streaming.controller.test.ts index 03b3274c870..ead68fddf48 100644 --- a/packages/cli/src/modules/log-streaming.ee/__tests__/log-streaming.controller.test.ts +++ b/packages/cli/src/modules/log-streaming.ee/__tests__/log-streaming.controller.test.ts @@ -9,6 +9,7 @@ import { BadRequestError } from '@/errors/response-errors/bad-request.error'; import { ForbiddenError } from '@/errors/response-errors/forbidden.error'; import type { MessageEventBus } from '@/eventbus/message-event-bus/message-event-bus'; +import type { AuditLogEventRepository } from '../database/repositories/audit-log-event.repository'; import type { LogStreamingDestinationService } from '../log-streaming-destination.service'; import { EventBusController } from '../log-streaming.controller'; @@ -19,6 +20,7 @@ describe('EventBusController', () => { logStreamingManagedByEnv: false, }); const outboundHttp = mock(); + const auditLogEventRepository = mock(); let controller: EventBusController; @@ -30,6 +32,7 @@ describe('EventBusController', () => { destinationService, instanceSettingsLoaderConfig, outboundHttp, + auditLogEventRepository, ); }); diff --git a/packages/cli/src/modules/log-streaming.ee/database/entities/audit-log-event.entity.ts b/packages/cli/src/modules/log-streaming.ee/database/entities/audit-log-event.entity.ts new file mode 100644 index 00000000000..06be45c2dc1 --- /dev/null +++ b/packages/cli/src/modules/log-streaming.ee/database/entities/audit-log-event.entity.ts @@ -0,0 +1,27 @@ +import { DateTimeColumn, JsonColumn, WithTimestamps } from '@n8n/db'; +import { Column, Entity, Index, PrimaryColumn } from '@n8n/typeorm'; + +/** + * Debug sink for MessageEventBus events. Populated by the `database` log-streaming + * destination when N8N_AUDIT_LOG_DB_SINK is enabled. PoC only: unbounded, no pruning. + */ +@Entity({ name: 'audit_log_event' }) +export class AuditLogEvent extends WithTimestamps { + // The originating EventMessage id (uuid); reused so re-delivery is idempotent. + @PrimaryColumn('varchar') + id: string; + + @Index() + @Column('varchar') + eventName: string; + + @Column('varchar') + message: string; + + // The event's own timestamp (distinct from createdAt, the ingest time). + @DateTimeColumn() + ts: Date; + + @JsonColumn({ nullable: true }) + payload: unknown; +} diff --git a/packages/cli/src/modules/log-streaming.ee/database/entities/index.ts b/packages/cli/src/modules/log-streaming.ee/database/entities/index.ts index 26a826e8984..b4899c89433 100644 --- a/packages/cli/src/modules/log-streaming.ee/database/entities/index.ts +++ b/packages/cli/src/modules/log-streaming.ee/database/entities/index.ts @@ -1 +1,2 @@ export { EventDestinations } from './event-destination.entity'; +export { AuditLogEvent } from './audit-log-event.entity'; diff --git a/packages/cli/src/modules/log-streaming.ee/database/repositories/__tests__/audit-log-event.repository.integration.test.ts b/packages/cli/src/modules/log-streaming.ee/database/repositories/__tests__/audit-log-event.repository.integration.test.ts new file mode 100644 index 00000000000..3a1c190a1bf --- /dev/null +++ b/packages/cli/src/modules/log-streaming.ee/database/repositories/__tests__/audit-log-event.repository.integration.test.ts @@ -0,0 +1,60 @@ +import { testDb, testModules } from '@n8n/backend-test-utils'; +import { Container } from '@n8n/di'; + +import { AuditLogEvent } from '../../entities/audit-log-event.entity'; +import { AuditLogEventRepository } from '../audit-log-event.repository'; + +const makeEvent = (id: string, eventName: string): AuditLogEvent => + Object.assign(new AuditLogEvent(), { + id, + eventName, + message: eventName, + ts: new Date(), + payload: {}, + }); + +describe('AuditLogEventRepository', () => { + let repository: AuditLogEventRepository; + + beforeAll(async () => { + await testModules.loadModules(['log-streaming']); + await testDb.init(); + repository = Container.get(AuditLogEventRepository); + }); + + beforeEach(async () => { + await repository.delete({}); + }); + + afterAll(async () => { + await testDb.terminate(); + }); + + it('should filter by event-name prefix and return the total count', async () => { + await repository.store(makeEvent('1', 'n8n.audit.user.login.success')); + await repository.store(makeEvent('2', 'n8n.audit.workflow.created')); + await repository.store(makeEvent('3', 'n8n.workflow.started')); + + const [audit, auditCount] = await repository.listByPrefix({ + prefix: 'n8n.audit', + skip: 0, + take: 10, + }); + expect(auditCount).toBe(2); + expect(audit.map((e) => e.id).sort()).toEqual(['1', '2']); + + const [all, allCount] = await repository.listByPrefix({ skip: 0, take: 10 }); + expect(allCount).toBe(3); + expect(all).toHaveLength(3); + }); + + it('should page through results', async () => { + await repository.store(makeEvent('1', 'n8n.audit.a')); + await repository.store(makeEvent('2', 'n8n.audit.b')); + await repository.store(makeEvent('3', 'n8n.audit.c')); + + const [page, count] = await repository.listByPrefix({ prefix: 'n8n.audit', skip: 0, take: 2 }); + expect(count).toBe(3); + expect(page).toHaveLength(2); + }); +}); diff --git a/packages/cli/src/modules/log-streaming.ee/database/repositories/audit-log-event.repository.ts b/packages/cli/src/modules/log-streaming.ee/database/repositories/audit-log-event.repository.ts new file mode 100644 index 00000000000..3b2152cbc96 --- /dev/null +++ b/packages/cli/src/modules/log-streaming.ee/database/repositories/audit-log-event.repository.ts @@ -0,0 +1,24 @@ +import { Service } from '@n8n/di'; +import { DataSource, Like, Repository } from '@n8n/typeorm'; + +import { AuditLogEvent } from '../entities/audit-log-event.entity'; + +@Service() +export class AuditLogEventRepository extends Repository { + constructor(dataSource: DataSource) { + super(AuditLogEvent, dataSource.manager); + } + + async store(event: AuditLogEvent) { + await this.save(event); + } + + async listByPrefix({ prefix, skip, take }: { prefix?: string; skip: number; take: number }) { + return await this.findAndCount({ + where: prefix ? { eventName: Like(`${prefix}%`) } : {}, + order: { ts: 'DESC' }, + skip, + take, + }); + } +} diff --git a/packages/cli/src/modules/log-streaming.ee/destinations/__tests__/message-event-bus-destination-database.ee.test.ts b/packages/cli/src/modules/log-streaming.ee/destinations/__tests__/message-event-bus-destination-database.ee.test.ts new file mode 100644 index 00000000000..108f97b8ffd --- /dev/null +++ b/packages/cli/src/modules/log-streaming.ee/destinations/__tests__/message-event-bus-destination-database.ee.test.ts @@ -0,0 +1,77 @@ +import { mockInstance } from '@n8n/backend-test-utils'; +import { MessageEventBusDestinationTypeNames } from 'n8n-workflow'; + +import type { MessageEventBus } from '@/eventbus/message-event-bus/message-event-bus'; + +import { AuditLogEventRepository } from '../../database/repositories/audit-log-event.repository'; +import { MessageEventBusDestinationDatabase } from '../message-event-bus-destination-database.ee'; + +const mockEventBus = {} as MessageEventBus; + +const createMessage = () => + ({ + id: 'event-1', + eventName: 'n8n.audit.user.login.success', + message: 'user logged in', + ts: { toJSDate: () => new Date('2020-01-01T00:00:00.000Z') }, + payload: { userId: 'u1' }, + anonymize: () => ({ anonymized: true }), + }) as any; + +const buildDestination = () => + new MessageEventBusDestinationDatabase(mockEventBus, { + __type: MessageEventBusDestinationTypeNames.database, + enabled: true, + subscribedEvents: ['*'], + }); + +describe('MessageEventBusDestinationDatabase', () => { + let repository: ReturnType>; + + beforeEach(() => { + vi.clearAllMocks(); + repository = mockInstance(AuditLogEventRepository); + repository.create.mockImplementation((row) => row as never); + }); + + it('should persist the mapped event and confirm delivery', async () => { + const confirmCallback = vi.fn(); + const destination = buildDestination(); + + const result = await destination.receiveFromEventBus({ + msg: createMessage(), + confirmCallback, + } as any); + + expect(result).toBe(true); + expect(repository.store).toHaveBeenCalledWith({ + id: 'event-1', + eventName: 'n8n.audit.user.login.success', + message: 'user logged in', + ts: new Date('2020-01-01T00:00:00.000Z'), + payload: { userId: 'u1' }, + }); + expect(confirmCallback).toHaveBeenCalledWith( + expect.objectContaining({ id: 'event-1' }), + expect.objectContaining({ name: 'Audit Log Database' }), + ); + }); + + it('should store the anonymized payload when anonymization is enabled', async () => { + const destination = new MessageEventBusDestinationDatabase(mockEventBus, { + __type: MessageEventBusDestinationTypeNames.database, + enabled: true, + subscribedEvents: ['*'], + anonymizeAuditMessages: true, + }); + + await destination.receiveFromEventBus({ + msg: createMessage(), + confirmCallback: vi.fn(), + } as any); + + expect(repository.store).toHaveBeenCalledWith( + expect.objectContaining({ payload: { anonymized: true } }), + ); + }); +}); diff --git a/packages/cli/src/modules/log-streaming.ee/destinations/message-event-bus-destination-database.ee.ts b/packages/cli/src/modules/log-streaming.ee/destinations/message-event-bus-destination-database.ee.ts new file mode 100644 index 00000000000..084f77f5f45 --- /dev/null +++ b/packages/cli/src/modules/log-streaming.ee/destinations/message-event-bus-destination-database.ee.ts @@ -0,0 +1,43 @@ +import { Container } from '@n8n/di'; +import { MessageEventBusDestinationTypeNames } from 'n8n-workflow'; +import type { MessageEventBusDestinationOptions } from 'n8n-workflow'; + +import type { + MessageEventBus, + MessageWithCallback, +} from '@/eventbus/message-event-bus/message-event-bus'; + +import { MessageEventBusDestination } from './message-event-bus-destination.ee'; +import { AuditLogEventRepository } from '../database/repositories/audit-log-event.repository'; + +/** + * Debug destination that persists every received event into the `audit_log_event` + * table. Auto-provisioned in-memory when N8N_AUDIT_LOG_DB_SINK=true; not stored in + * the destinations config, so it needs no serialize/deserialize. + */ +export class MessageEventBusDestinationDatabase extends MessageEventBusDestination { + constructor(eventBusInstance: MessageEventBus, options: MessageEventBusDestinationOptions) { + super(eventBusInstance, options); + this.label = options.label ?? 'Audit Log Database'; + this.__type = MessageEventBusDestinationTypeNames.database; + } + + async receiveFromEventBus(emitterPayload: MessageWithCallback): Promise { + const { msg, confirmCallback } = emitterPayload; + const payload = this.anonymizeAuditMessages ? msg.anonymize() : msg.payload; + + const repository = Container.get(AuditLogEventRepository); + await repository.store( + repository.create({ + id: msg.id, + eventName: msg.eventName, + message: msg.message, + ts: msg.ts.toJSDate(), + payload, + }), + ); + + confirmCallback(msg, { id: this.id, name: this.label }); + return true; + } +} diff --git a/packages/cli/src/modules/log-streaming.ee/log-streaming-destination.service.ts b/packages/cli/src/modules/log-streaming.ee/log-streaming-destination.service.ts index e066e65c38f..e316bae3034 100644 --- a/packages/cli/src/modules/log-streaming.ee/log-streaming-destination.service.ts +++ b/packages/cli/src/modules/log-streaming.ee/log-streaming-destination.service.ts @@ -91,6 +91,15 @@ export class LogStreamingDestinationService { return await this.eventDestinationsRepository.delete({ id }); } + /** + * Register a destination in the local map only, without DB persistence or pubsub. + * Used for env-provisioned debug destinations that are recreated on every boot. + */ + addInMemoryDestination(destination: MessageEventBusDestination): void { + this.destinations[destination.getId()] = destination; + this.logger.debug(`Added in-memory destination ${destination.getId()}`); + } + /** * Add a destination to the local map and save to database */ diff --git a/packages/cli/src/modules/log-streaming.ee/log-streaming.controller.ts b/packages/cli/src/modules/log-streaming.ee/log-streaming.controller.ts index 41d4e4759c4..34aea5188c9 100644 --- a/packages/cli/src/modules/log-streaming.ee/log-streaming.controller.ts +++ b/packages/cli/src/modules/log-streaming.ee/log-streaming.controller.ts @@ -2,6 +2,7 @@ import { CreateDestinationDto, DeleteDestinationQueryDto, GetDestinationQueryDto, + ListAuditLogEventsQueryDto, TestDestinationQueryDto, } from '@n8n/api-types'; import { OutboundHttp } from '@n8n/backend-network'; @@ -16,6 +17,8 @@ import { eventNamesAll } from '@/eventbus/event-message-classes'; import { MessageEventBus } from '@/eventbus/message-event-bus/message-event-bus'; import { createMessageEventBusDestination } from './create-message-event-bus-destination'; +import type { AuditLogEvent } from './database/entities/audit-log-event.entity'; +import { AuditLogEventRepository } from './database/repositories/audit-log-event.repository'; import { LogStreamingDestinationService } from './log-streaming-destination.service'; @RestController('/eventbus') @@ -25,6 +28,7 @@ export class EventBusController { private readonly destinationService: LogStreamingDestinationService, private readonly instanceSettingsLoaderConfig: InstanceSettingsLoaderConfig, private readonly outboundHttp: OutboundHttp, + private readonly auditLogEventRepository: AuditLogEventRepository, ) {} private assertNotManagedByEnv() { @@ -40,6 +44,21 @@ export class EventBusController { return eventNamesAll; } + @Get('/audit-log-events') + @GlobalScope('eventBusDestination:list') + async listAuditLogEvents( + _req: AuthenticatedRequest, + _res: unknown, + @Query query: ListAuditLogEventsQueryDto, + ): Promise<{ data: AuditLogEvent[]; count: number }> { + const [data, count] = await this.auditLogEventRepository.listByPrefix({ + prefix: query.prefix, + skip: query.skip, + take: query.take, + }); + return { data, count }; + } + @Licensed('feat:logStreaming') @Get('/destination') @GlobalScope('eventBusDestination:list') diff --git a/packages/cli/src/modules/log-streaming.ee/log-streaming.module.ts b/packages/cli/src/modules/log-streaming.ee/log-streaming.module.ts index 61069f45c1d..05cb89ca865 100644 --- a/packages/cli/src/modules/log-streaming.ee/log-streaming.module.ts +++ b/packages/cli/src/modules/log-streaming.ee/log-streaming.module.ts @@ -2,6 +2,7 @@ import { LICENSE_FEATURES } from '@n8n/constants'; import type { ModuleInterface } from '@n8n/decorators'; import { BackendModule } from '@n8n/decorators'; import { Container } from '@n8n/di'; +import { MessageEventBusDestinationTypeNames } from 'n8n-workflow'; /** * Log Streaming module provides enterprise-grade event logging @@ -25,10 +26,28 @@ export class LogStreamingModule implements ModuleInterface { const destinationService = Container.get(LogStreamingDestinationService); await destinationService.loadDestinationsFromDb(); await destinationService.initialize(); + + // Debug sink: persist every event to the audit_log_event table. Opt-in and + // recreated on every boot, so it lives in-memory only (no config persistence). + if (process.env.N8N_AUDIT_LOG_DB_SINK === 'true') { + const { MessageEventBusDestinationDatabase } = await import( + './destinations/message-event-bus-destination-database.ee.js' + ); + const { MessageEventBus } = await import('@/eventbus/message-event-bus/message-event-bus.js'); + destinationService.addInMemoryDestination( + new MessageEventBusDestinationDatabase(Container.get(MessageEventBus), { + __type: MessageEventBusDestinationTypeNames.database, + label: 'Audit Log Database', + enabled: true, + subscribedEvents: ['*'], + }), + ); + } } async entities() { const { EventDestinations } = await import('./database/entities/event-destination.entity.js'); - return [EventDestinations]; + const { AuditLogEvent } = await import('./database/entities/audit-log-event.entity.js'); + return [EventDestinations, AuditLogEvent]; } } diff --git a/packages/cli/src/modules/oauth-server/__tests__/agent-mint-hook.api.test.ts b/packages/cli/src/modules/oauth-server/__tests__/agent-mint-hook.api.test.ts index d82d4e0d4c1..cbaf5cdffcb 100644 --- a/packages/cli/src/modules/oauth-server/__tests__/agent-mint-hook.api.test.ts +++ b/packages/cli/src/modules/oauth-server/__tests__/agent-mint-hook.api.test.ts @@ -215,9 +215,10 @@ describe('agent self-mint across the real HTTP token boundary', () => { aud: resourceUrl, outcome: 'success', }); - // ...and the verify site emits a matching verified event. + // ...and the verify site emits a matching verified event (autonomous: no actor). expect(emitSpy).toHaveBeenCalledWith('service-account-token-verified', { sub: saUserId, + act: null, aud: resourceUrl, outcome: 'success', }); diff --git a/packages/cli/src/modules/oauth-server/__tests__/agent-obo-mint.api.test.ts b/packages/cli/src/modules/oauth-server/__tests__/agent-obo-mint.api.test.ts new file mode 100644 index 00000000000..da22a8d0a06 --- /dev/null +++ b/packages/cli/src/modules/oauth-server/__tests__/agent-obo-mint.api.test.ts @@ -0,0 +1,301 @@ +// The service-accounts feature is gated on this env flag, read at module-init +// time and again inside `AgentsService.create`. It must be set before any +// `setupTestServer` beforeAll (module init) or agent-provisioning code runs, so +// it is set at module scope and restored in afterAll. +const priorServiceAccountsFlag = process.env.N8N_ENV_FEAT_SERVICE_ACCOUNTS; +process.env.N8N_ENV_FEAT_SERVICE_ACCOUNTS = 'true'; + +import { OutboundHttp } from '@n8n/backend-network'; +import { + createWorkflowWithHistory, + getPersonalProject, + setActiveVersion, + testDb, +} from '@n8n/backend-test-utils'; +import { GlobalConfig } from '@n8n/config'; +import type { User } from '@n8n/db'; +import { WebhookRepository } from '@n8n/db'; +import { Container } from '@n8n/di'; +import type { INode } from 'n8n-workflow'; +import { randomUUID } from 'node:crypto'; +import type { AddressInfo } from 'node:net'; + +import { MCP_TRIGGER_NODE_TYPE } from '@/constants'; +import { AgentsService } from '@/modules/agents/agents.service'; +import { AgentRepository } from '@/modules/agents/repositories/agent.repository'; +import { InternalOAuth2MintService } from '@/modules/oauth-server/internal-oauth2-mint.service'; +import { OAuthTokenService } from '@/modules/oauth-server/oauth-token.service'; +import { CacheService } from '@/services/cache/cache.service'; +import { JwtService } from '@/services/jwt.service'; +import { UrlService } from '@/services/url.service'; +import { createOwner } from '@test-integration/db/users'; +import { setupTestServer } from '@test-integration/utils'; + +/** + * GATE-OBO — the delegated (on-behalf-of) mint proof across the real HTTP token + * boundary. Where the autonomous proof (`agent-mint-hook.api.test.ts`) mints a + * `client_credentials` token whose `sub` is the agent's service account, this + * exercises the RFC 8693 token-exchange path: a human-triggered agent run mints + * a *delegated* token via `InternalOAuth2MintService.mintForUser(..., onBehalfOfUserId)`, + * which self-mints a subject assertion for the human and performs a real HTTP + * token-exchange self-call against this live test server's own `/oauth/token`. + * + * The minted token carries `{ sub: human, act: { sub: serviceAccount } }`, the + * MCP-trigger verify resolves BOTH identities, and — the point — authorizes + * against the SUBJECT (the human), not the actor. A fresh service account that + * lacks execute on the human's workflow is admitted when it acts on the human's + * behalf, yet denied when it mints autonomously for the same resource: proof + * that OBO authorizes as the human. + */ +const testServer = setupTestServer({ + endpointGroups: ['mcp'], + modules: ['oauth-server', 'mcp', 'agents', 'service-accounts'], +}); + +// The human subject: an owner, so she holds `workflow:execute` via her global role. +let alice: User; +let projectId: string; + +let agentId: string; +let saUserId: string; +let priorEditorBaseUrl: string; +let priorWebhookUrl: string; + +const mcpEndpoint = () => Container.get(GlobalConfig).endpoints.mcp; +const webhookBaseUrl = () => Container.get(UrlService).getWebhookBaseUrl().replace(/\/$/, ''); +const productionResourceUrl = (webhookPath: string) => + `${webhookBaseUrl()}/${mcpEndpoint()}/${webhookPath}`; + +const mcpTriggerNode = (): INode => ({ + id: randomUUID(), + name: 'MCP Server Trigger', + type: MCP_TRIGGER_NODE_TYPE, + typeVersion: 2, + position: [0, 0], + parameters: { + path: 'unused', + authentication: 'n8nOAuth2', + // Secure default: execute access is required (the param is omitted so the + // "absent param ⇒ require execute" path runs). This is what makes the + // subject-vs-actor authorization differential observable. + }, +}); + +/** + * Create an active, published workflow owned by `alice` whose MCP trigger is + * protected with n8n OAuth2, plus the production webhook row the resolver + * matches on. Returns the canonical resource URL the OAuth server binds tokens + * and consent to. + */ +const createProtectedWorkflow = async (workflowName: string) => { + const node = mcpTriggerNode(); + const webhookPath = randomUUID(); + const workflow = await createWorkflowWithHistory( + { name: workflowName, active: true, nodes: [node] }, + alice, + ); + await setActiveVersion(workflow.id, workflow.versionId); + await Container.get(WebhookRepository).insert({ + workflowId: workflow.id, + webhookPath, + method: 'POST', + node: node.name, + }); + return { workflow, resourceUrl: productionResourceUrl(webhookPath) }; +}; + +beforeAll(async () => { + // The mint discovers its token endpoint from n8n's own OAuth2 metadata, fetched + // at the protected resource's own origin (the webhook base URL) and the issuer it + // advertises (the instance base URL). Point BOTH at the live ephemeral port so + // discovery — and the subsequent token-exchange self-call — reach THIS test + // server rather than the default :5678 host, which nothing is listening on in the + // vitest environment. Overriding only `editorBaseUrl` would leave the resource URL + // (built from the webhook base URL) pointing at :5678, so discovery would fail and + // silently exercise the fallback path instead of the real discovered one. + const address = testServer.httpServer.address() as AddressInfo; + const liveBaseUrl = `http://127.0.0.1:${address.port}`; + const globalConfig = Container.get(GlobalConfig); + priorEditorBaseUrl = globalConfig.editorBaseUrl; + priorWebhookUrl = globalConfig.webhookUrl; + globalConfig.editorBaseUrl = liveBaseUrl; + globalConfig.webhookUrl = liveBaseUrl; + + alice = await createOwner(); + projectId = (await getPersonalProject(alice)).id; + + // Provision the agent. With the feature flag enabled, `create` eagerly + // provisions the 1:1 service account; fall back to the lazy backfill if it + // did not fire, so the rest of the proof still runs. The SA is a FRESH + // `global:member` user that is NOT shared into Alice's workflow, so it lacks + // `workflow:execute` on the protected resource — which is what the killer + // assertion below turns on. + const agent = await Container.get(AgentsService).create(projectId, 'OBO Mint Agent'); + agentId = agent.id; + + const reloaded = await Container.get(AgentRepository).findByIdAndProjectId(agentId, projectId); + saUserId = + reloaded?.serviceAccountUserId ?? + (await Container.get(AgentsService).getOrCreateServiceAccountUserId(agent)); +}); + +afterEach(async () => { + await Container.get(CacheService).reset(); + // Only per-test resources are cleared. The agent, its service account, and + // its (encrypted) client credential survive across tests so the SA identity + // stays stable; the SA client's shadow oauth_clients row is re-created + // idempotently by the next exchange. + await testDb.truncate([ + 'AccessToken', + 'RefreshToken', + 'AuthorizationCode', + 'OAuthClient', + 'UserConsent', + 'WebhookEntity', + 'SharedWorkflow', + 'WorkflowEntity', + 'WorkflowHistory', + ]); +}); + +afterAll(() => { + Container.get(GlobalConfig).editorBaseUrl = priorEditorBaseUrl; + Container.get(GlobalConfig).webhookUrl = priorWebhookUrl; + if (priorServiceAccountsFlag === undefined) { + delete process.env.N8N_ENV_FEAT_SERVICE_ACCOUNTS; + } else { + process.env.N8N_ENV_FEAT_SERVICE_ACCOUNTS = priorServiceAccountsFlag; + } +}); + +describe('delegated (on-behalf-of) mint across the real HTTP token boundary', () => { + // Proves the mint takes the *discovered* branch, not the fallback: the mint runs + // exactly this `resolveResourceAuth` logic (RFC 9728 → RFC 8414) before minting, + // and the fallback is only reached when discovery throws. Resolving here against + // the live server for the fixture resource — and pinning the advertised + // `/mcp-oauth/token` endpoint — confirms discovery succeeds, so the real + // discovered token-exchange path is what runs end-to-end below. + test('resolves the fixture resource through real OAuth2 discovery (RFC 9728 → RFC 8414)', async () => { + const { resourceUrl } = await createProtectedWorkflow('OBO discovery fixture'); + const target = new URL(resourceUrl); + const http = Container.get(OutboundHttp).requests({ ssrf: 'disabled' }); + + const prm = await http.request<{ resource?: string; authorization_servers?: string[] }>({ + url: `${target.origin}/.well-known/oauth-protected-resource${target.pathname}${target.search}`, + method: 'GET', + }); + expect(prm.resource).toBe(resourceUrl); + const issuer = prm.authorization_servers?.[0]; + expect(issuer).toBeTruthy(); + + const asMetadata = await http.request<{ token_endpoint?: string }>({ + url: `${issuer}/.well-known/oauth-authorization-server`, + method: 'GET', + }); + expect(asMetadata.token_endpoint).toBe(`${issuer}/mcp-oauth/token`); + }); + + test('mints a delegated token whose claims name the human as subject and the service account as actor', async () => { + const { resourceUrl } = await createProtectedWorkflow('OBO delegated claims'); + + // Real HTTP token-exchange self-call: the acting SA mints on behalf of Alice. + const token = await Container.get(InternalOAuth2MintService).mintForUser( + saUserId, + resourceUrl, + {}, + alice.id, + ); + expect(token).toBeTruthy(); + + const decoded = Container.get(JwtService).decode<{ + sub?: string; + aud?: string; + act?: { sub?: string }; + }>(token); + + // RFC 8693 delegation shape: subject is the human, actor is the acting SA, + // audience is the discovered canonical resource. + expect(decoded.sub).toBe(alice.id); + expect(decoded.act?.sub).toBe(saUserId); + expect(decoded.aud).toBe(resourceUrl); + }); + + test('verify resolves both identities and authorizes against the subject', async () => { + const { resourceUrl } = await createProtectedWorkflow('OBO verify resolves both'); + + const token = await Container.get(InternalOAuth2MintService).mintForUser( + saUserId, + resourceUrl, + {}, + alice.id, + ); + + const result = await Container.get(OAuthTokenService).verifyOAuthAccessToken( + token, + resourceUrl, + ); + + // The subject (Alice) is the authorized principal; the actor (SA) is + // surfaced for attribution. Alice holds execute via her global role, so + // the delegated token is accepted. + expect(result.user?.id).toBe(alice.id); + expect(result.actor?.id).toBe(saUserId); + }); + + test('authorization follows the subject, not the actor', async () => { + const { resourceUrl } = await createProtectedWorkflow('OBO subject-vs-actor'); + + // Delegated as Alice (owner, has execute): accepted, resolves to Alice. + const delegatedToken = await Container.get(InternalOAuth2MintService).mintForUser( + saUserId, + resourceUrl, + {}, + alice.id, + ); + const delegated = await Container.get(OAuthTokenService).verifyOAuthAccessToken( + delegatedToken, + resourceUrl, + ); + expect(delegated.user?.id).toBe(alice.id); + expect(delegated.actor?.id).toBe(saUserId); + + // Autonomous as the SA itself (same SA, same resource) — the SA lacks + // execute on Alice's workflow, so verify denies it. + const autonomousToken = await Container.get(InternalOAuth2MintService).mintForUser( + saUserId, + resourceUrl, + ); + const autonomous = await Container.get(OAuthTokenService).verifyOAuthAccessToken( + autonomousToken, + resourceUrl, + ); + expect(autonomous.user).toBeNull(); + expect(autonomous.context?.reason).toBe('insufficient_scope'); + }); + + test('rejects the delegated token at a different audience (audience isolation)', async () => { + const { resourceUrl: resourceUrlA } = await createProtectedWorkflow('OBO audience A'); + const { resourceUrl: resourceUrlB } = await createProtectedWorkflow('OBO audience B'); + + const token = await Container.get(InternalOAuth2MintService).mintForUser( + saUserId, + resourceUrlA, + {}, + alice.id, + ); + + // Its own audience: accepted and resolved to Alice. + const acceptedAtA = await Container.get(OAuthTokenService).verifyOAuthAccessToken( + token, + resourceUrlA, + ); + expect(acceptedAtA.user?.id).toBe(alice.id); + + // A different resource's audience: rejected (audience isolation). + const rejectedAtB = await Container.get(OAuthTokenService).verifyOAuthAccessToken( + token, + resourceUrlB, + ); + expect(rejectedAtB.user).toBeNull(); + }); +}); diff --git a/packages/cli/src/modules/oauth-server/__tests__/internal-oauth2-mint.service.test.ts b/packages/cli/src/modules/oauth-server/__tests__/internal-oauth2-mint.service.test.ts index 711d77ec8fb..c503b69cf35 100644 --- a/packages/cli/src/modules/oauth-server/__tests__/internal-oauth2-mint.service.test.ts +++ b/packages/cli/src/modules/oauth-server/__tests__/internal-oauth2-mint.service.test.ts @@ -13,6 +13,7 @@ import type { ServiceAccountCredentialService } from '@/services/service-account import type { UrlService } from '@/services/url.service'; import { InternalOAuth2MintService } from '../internal-oauth2-mint.service'; +import type { OAuthTokenService } from '../oauth-token.service'; const ORIGIN = 'http://localhost:5678'; const TARGET_URL = `${ORIGIN}/mcp/proof`; @@ -52,10 +53,17 @@ describe('InternalOAuth2MintService', () => { const urlService = mock(); const eventService = mock(); const logger = mock(); + const oauthTokenService = mock(); const request = vi.fn(); const requests = vi.fn().mockReturnValue(mock({ request })); const outboundHttp = mock({ requests }); + const SUBJECT_ASSERTION = 'subject-assertion-jwt'; + const HUMAN_USER_ID = 'human-user-id'; + // The SA's OWN access token, minted via client_credentials, then presented as + // the RFC 8693 actor_token in the delegated exchange. + const SA_ACCESS_TOKEN = 'sa-access-token'; + let service: InternalOAuth2MintService; beforeEach(() => { @@ -65,11 +73,13 @@ describe('InternalOAuth2MintService', () => { clientId: CLIENT_ID, clientSecret: CLIENT_SECRET, }); + oauthTokenService.mintSubjectAssertion.mockReturnValue(SUBJECT_ASSERTION); service = new InternalOAuth2MintService( serviceAccountCredentialService, urlService, eventService, logger, + oauthTokenService, outboundHttp, ); }); @@ -77,6 +87,14 @@ describe('InternalOAuth2MintService', () => { const findCall = (url: string) => request.mock.calls.map(([options]) => options as RequestOptions).find((o) => o.url === url); + // Both delegated POSTs hit the same discovered endpoint; distinguish them by grant. + const mintCalls = () => + request.mock.calls + .map(([options]) => options as RequestOptions) + .filter((o) => o.url === DISCOVERED_TOKEN_ENDPOINT); + const findMintCall = (grantType: string) => + mintCalls().find((o) => new URLSearchParams(o.body).get('grant_type') === grantType); + describe('protocol-driven discovery', () => { beforeEach(() => { // Route by URL: PRM well-known → RFC 9728 doc, AS well-known → RFC 8414 doc, @@ -161,6 +179,129 @@ describe('InternalOAuth2MintService', () => { }); }); + describe('delegated (on-behalf-of) mint', () => { + const TOKEN_EXCHANGE_GRANT = 'urn:ietf:params:oauth:grant-type:token-exchange'; + + beforeEach(() => { + // Both delegated POSTs hit the discovered endpoint; route by grant: the + // client_credentials call returns the SA's own token, the token-exchange + // call (which must carry that token as actor_token) returns the final token. + request.mockImplementation(({ url, body }: RequestOptions) => { + if (url === PRM_WELL_KNOWN) { + return { resource: CANONICAL_RESOURCE, authorization_servers: [ISSUER] }; + } + if (url === AS_WELL_KNOWN) return { token_endpoint: DISCOVERED_TOKEN_ENDPOINT }; + if (url === DISCOVERED_TOKEN_ENDPOINT) { + const grant = new URLSearchParams(body).get('grant_type'); + if (grant === 'client_credentials') return { access_token: SA_ACCESS_TOKEN }; + if (grant === TOKEN_EXCHANGE_GRANT) return { access_token: 'delegated-token' }; + } + throw new Error(`unexpected request url: ${url}`); + }); + }); + + it('mints the SA access token, then exchanges it as the actor_token for the human', async () => { + const token = await service.mintForUser(USER_ID, TARGET_URL, {}, HUMAN_USER_ID); + + expect(token).toBe('delegated-token'); + + // The subject assertion is minted for the human (never the acting SA). + expect(oauthTokenService.mintSubjectAssertion).toHaveBeenCalledWith(HUMAN_USER_ID); + + // Step 1: the SA obtained its OWN access token via client_credentials. + const ccCall = findMintCall('client_credentials'); + expect(ccCall?.method).toBe('POST'); + const ccParams = new URLSearchParams(ccCall?.body); + expect(ccParams.get('client_id')).toBe(CLIENT_ID); + expect(ccParams.get('client_secret')).toBe(CLIENT_SECRET); + expect(ccParams.get('resource')).toBe(CANONICAL_RESOURCE); + + // Step 2: the token-exchange POST carries that token as the RFC 8693 actor_token. + const exchangeCall = findMintCall(TOKEN_EXCHANGE_GRANT); + expect(exchangeCall?.method).toBe('POST'); + const params = new URLSearchParams(exchangeCall?.body); + expect(params.get('grant_type')).toBe(TOKEN_EXCHANGE_GRANT); + expect(params.get('subject_token')).toBe(SUBJECT_ASSERTION); + expect(params.get('subject_token_type')).toBe('urn:ietf:params:oauth:token-type:jwt'); + // The actor_token is exactly the token the client_credentials call returned. + expect(params.get('actor_token')).toBe(SA_ACCESS_TOKEN); + expect(params.get('actor_token_type')).toBe('urn:ietf:params:oauth:token-type:access_token'); + // The actor SA's own client credentials still authenticate the exchange. + expect(params.get('client_id')).toBe(CLIENT_ID); + expect(params.get('client_secret')).toBe(CLIENT_SECRET); + expect(params.get('resource')).toBe(CANONICAL_RESOURCE); + + // Both grants ran: client_credentials first, token-exchange second. + expect(mintCalls().map((o) => new URLSearchParams(o.body).get('grant_type'))).toEqual([ + 'client_credentials', + TOKEN_EXCHANGE_GRANT, + ]); + + // The delegated mint audit records the human as `sub` and the acting SA as + // the RFC 8693 `act`, keyed on the canonical resource. + expect(eventService.emit).toHaveBeenCalledWith('service-account-token-minted', { + sub: HUMAN_USER_ID, + act: USER_ID, + clientId: CLIENT_ID, + aud: CANONICAL_RESOURCE, + outcome: 'success', + }); + }); + + it('mints autonomously (single client_credentials call, no assertion) when no human is present', async () => { + const token = await service.mintForUser(USER_ID, TARGET_URL); + + expect(token).toBe(SA_ACCESS_TOKEN); + expect(oauthTokenService.mintSubjectAssertion).not.toHaveBeenCalled(); + // A single client_credentials POST — no token-exchange call. + expect(mintCalls().map((o) => new URLSearchParams(o.body).get('grant_type'))).toEqual([ + 'client_credentials', + ]); + expect(findMintCall('client_credentials')?.body).toContain('grant_type=client_credentials'); + expect( + new URLSearchParams(findMintCall('client_credentials')?.body).get('subject_token'), + ).toBeNull(); + + // An autonomous mint records the acting SA as `sub` with no `act`. + expect(eventService.emit).toHaveBeenCalledWith('service-account-token-minted', { + sub: USER_ID, + clientId: CLIENT_ID, + aud: CANONICAL_RESOURCE, + outcome: 'success', + }); + }); + + it('emits failure and does not attempt the exchange when the client_credentials step fails', async () => { + request.mockImplementation(({ url, body }: RequestOptions) => { + if (url === PRM_WELL_KNOWN) { + return { resource: CANONICAL_RESOURCE, authorization_servers: [ISSUER] }; + } + if (url === AS_WELL_KNOWN) return { token_endpoint: DISCOVERED_TOKEN_ENDPOINT }; + if (url === DISCOVERED_TOKEN_ENDPOINT) { + const grant = new URLSearchParams(body).get('grant_type'); + if (grant === 'client_credentials') throw http401(); + } + throw new Error(`unexpected request url: ${url}`); + }); + + await expect(service.mintForUser(USER_ID, TARGET_URL, {}, HUMAN_USER_ID)).rejects.toThrow( + OperationalError, + ); + + // The exchange never ran, and no subject assertion was minted for it. + expect(findMintCall('urn:ietf:params:oauth:grant-type:token-exchange')).toBeUndefined(); + expect(oauthTokenService.mintSubjectAssertion).not.toHaveBeenCalled(); + // A delegated failure still records the human as `sub` and the SA as `act`. + expect(eventService.emit).toHaveBeenCalledWith('service-account-token-minted', { + sub: HUMAN_USER_ID, + act: USER_ID, + clientId: CLIENT_ID, + aud: CANONICAL_RESOURCE, + outcome: 'failure', + }); + }); + }); + describe('fallback when the target is not an n8n-served protected resource', () => { it('warns and mints against the instance token endpoint with the raw target URL', async () => { // PRM discovery 404s → fall back to `{instanceBaseUrl}/oauth/token`. diff --git a/packages/cli/src/modules/oauth-server/__tests__/oauth-server.service.test.ts b/packages/cli/src/modules/oauth-server/__tests__/oauth-server.service.test.ts index 63d013a8a0e..cd59a365eee 100644 --- a/packages/cli/src/modules/oauth-server/__tests__/oauth-server.service.test.ts +++ b/packages/cli/src/modules/oauth-server/__tests__/oauth-server.service.test.ts @@ -157,6 +157,174 @@ describe('OAuthServerService', () => { }); }); + describe('exchangeToken', () => { + const clientId = 'sa-client-1'; + const rawSecret = 'correct-raw-secret'; + // The client's own service-account user. Deliberately DIFFERENT from the + // actor_token's subject below, to prove `act` follows the actor_token — not + // the authenticating client. + const clientSaUserId = 'sa-user-1'; + const subjectUserId = 'human-1'; + const actorTokenSub = 'actor-from-token'; + const subjectToken = 'subject-assertion-jwt'; + const actorToken = 'actor-access-token-jwt'; + + beforeEach(() => { + oauthClientRepository.findOneBy.mockResolvedValue({ id: clientId } as OAuthClient); + tokenService.getAccessTokenExpirySeconds.mockReturnValue(3600); + tokenService.generateAccessTokenOnly.mockResolvedValue({ + accessToken: 'delegated-token', + jti: 'delegated-jti', + }); + tokenService.validateSubjectAssertion.mockReturnValue(subjectUserId); + tokenService.validateActorToken.mockResolvedValue(actorTokenSub); + }); + + it('mints a delegated token whose act follows the actor_token, not the client', async () => { + serviceAccountCredentialServiceMock.getDecryptedByClientId.mockResolvedValue({ + credential: mock({ userId: clientSaUserId }), + clientSecret: rawSecret, + }); + + const result = await service.exchangeToken( + clientId, + rawSecret, + subjectToken, + actorToken, + TEST_RESOURCE_URL, + ); + + expect(result).toMatchObject({ + access_token: 'delegated-token', + token_type: 'Bearer', + expires_in: 3600, + }); + expect(tokenService.validateSubjectAssertion).toHaveBeenCalledWith(subjectToken); + expect(tokenService.validateActorToken).toHaveBeenCalledWith(actorToken); + // subject as `sub`, resource as `aud`, and the actor_token's subject as the + // delegated `act.sub` — NOT the authenticating client's SA user. + expect(tokenService.generateAccessTokenOnly).toHaveBeenCalledWith( + subjectUserId, + clientId, + TEST_RESOURCE_URL, + [], + actorTokenSub, + ); + expect(tokenService.generateAccessTokenOnly).not.toHaveBeenCalledWith( + subjectUserId, + clientId, + TEST_RESOURCE_URL, + [], + clientSaUserId, + ); + + // The delegation is audited with the human subject and the actor_token's subject. + expect(eventService.emit).toHaveBeenCalledWith('service-account-token-exchanged', { + sub: subjectUserId, + act: actorTokenSub, + clientId, + aud: TEST_RESOURCE_URL, + outcome: 'success', + }); + }); + + it('returns null for an unknown client without validating any token', async () => { + serviceAccountCredentialServiceMock.getDecryptedByClientId.mockResolvedValue(null); + + const result = await service.exchangeToken( + clientId, + rawSecret, + subjectToken, + actorToken, + TEST_RESOURCE_URL, + ); + + expect(result).toBeNull(); + expect(tokenService.validateSubjectAssertion).not.toHaveBeenCalled(); + expect(tokenService.validateActorToken).not.toHaveBeenCalled(); + expect(tokenService.generateAccessTokenOnly).not.toHaveBeenCalled(); + // Failure is audited with the known clientId and null sub/act. + expect(eventService.emit).toHaveBeenCalledWith('service-account-token-exchanged', { + sub: null, + act: null, + clientId, + aud: TEST_RESOURCE_URL, + outcome: 'failure', + }); + }); + + it('returns null for a wrong client secret without validating any token', async () => { + serviceAccountCredentialServiceMock.getDecryptedByClientId.mockResolvedValue({ + credential: mock({ userId: clientSaUserId }), + clientSecret: rawSecret, + }); + + const result = await service.exchangeToken( + clientId, + 'wrong-secret', + subjectToken, + actorToken, + TEST_RESOURCE_URL, + ); + + expect(result).toBeNull(); + expect(tokenService.validateSubjectAssertion).not.toHaveBeenCalled(); + expect(tokenService.validateActorToken).not.toHaveBeenCalled(); + expect(tokenService.generateAccessTokenOnly).not.toHaveBeenCalled(); + expect(eventService.emit).toHaveBeenCalledWith('service-account-token-exchanged', { + sub: null, + act: null, + clientId, + aud: TEST_RESOURCE_URL, + outcome: 'failure', + }); + }); + + it('rejects (throws) when the subject token is invalid, without minting', async () => { + serviceAccountCredentialServiceMock.getDecryptedByClientId.mockResolvedValue({ + credential: mock({ userId: clientSaUserId }), + clientSecret: rawSecret, + }); + tokenService.validateSubjectAssertion.mockImplementation(() => { + throw new Error('bad subject token'); + }); + + await expect( + service.exchangeToken(clientId, rawSecret, subjectToken, actorToken, TEST_RESOURCE_URL), + ).rejects.toThrow('bad subject token'); + expect(tokenService.generateAccessTokenOnly).not.toHaveBeenCalled(); + // The subject never resolved, so both sub and act are null. + expect(eventService.emit).toHaveBeenCalledWith('service-account-token-exchanged', { + sub: null, + act: null, + clientId, + aud: TEST_RESOURCE_URL, + outcome: 'failure', + }); + }); + + it('rejects (throws) when the actor token is invalid, without minting', async () => { + serviceAccountCredentialServiceMock.getDecryptedByClientId.mockResolvedValue({ + credential: mock({ userId: clientSaUserId }), + clientSecret: rawSecret, + }); + tokenService.validateActorToken.mockRejectedValue(new Error('bad actor token')); + + await expect( + service.exchangeToken(clientId, rawSecret, subjectToken, actorToken, TEST_RESOURCE_URL), + ).rejects.toThrow('bad actor token'); + expect(tokenService.generateAccessTokenOnly).not.toHaveBeenCalled(); + // The subject resolved before the actor token failed, so `sub` is known, `act` null. + expect(eventService.emit).toHaveBeenCalledWith('service-account-token-exchanged', { + sub: subjectUserId, + act: null, + clientId, + aud: TEST_RESOURCE_URL, + outcome: 'failure', + }); + }); + }); + describe('clientsStore', () => { describe('getClient', () => { it('should return client information when client exists', async () => { diff --git a/packages/cli/src/modules/oauth-server/__tests__/oauth-token.service.test.ts b/packages/cli/src/modules/oauth-server/__tests__/oauth-token.service.test.ts index 25ab5cb7847..9b0de595597 100644 --- a/packages/cli/src/modules/oauth-server/__tests__/oauth-token.service.test.ts +++ b/packages/cli/src/modules/oauth-server/__tests__/oauth-token.service.test.ts @@ -145,6 +145,127 @@ describe('OAuthTokenService', () => { expect(pair1.accessToken).not.toBe(pair2.accessToken); expect(pair1.refreshToken).not.toBe(pair2.refreshToken); }); + + it('omits the act claim for an autonomous (no-actor) token', () => { + const { accessToken } = service.generateTokenPair('user-123', 'client-456', undefined, []); + + expect(jwtService.decode(accessToken).act).toBeUndefined(); + }); + + it('includes an RFC 8693 act.sub claim for a delegated token', () => { + const { accessToken } = service.generateTokenPair( + 'subject-1', + 'client-456', + undefined, + [], + 'actor-1', + ); + + const decoded = jwtService.decode(accessToken); + expect(decoded.sub).toBe('subject-1'); + expect(decoded.act).toEqual({ sub: 'actor-1' }); + }); + }); + + describe('subject assertions (token exchange)', () => { + it('round-trips a subject assertion through its validator', () => { + const token = service.mintSubjectAssertion('human-1'); + + expect(service.validateSubjectAssertion(token)).toBe('human-1'); + + // It is deliberately not an access token: purpose-scoped, no isOAuth + // marker, and no oauth_access_tokens row persisted. + const decoded = jwtService.decode(token); + expect(decoded.purpose).toBe('subject_token'); + expect(decoded.meta).toBeUndefined(); + expect(accessTokenRepository.insertToken).not.toHaveBeenCalled(); + }); + + it('rejects a tampered assertion', () => { + const token = service.mintSubjectAssertion('human-1'); + + expect(() => service.validateSubjectAssertion(`${token}tampered`)).toThrow( + 'JWT Verification Failed', + ); + }); + + it('rejects an expired assertion', () => { + const expired = jwtService.sign({ sub: 'human-1', purpose: 'subject_token', iat: 0, exp: 1 }); + + expect(() => service.validateSubjectAssertion(expired)).toThrow('JWT Verification Failed'); + }); + + it('rejects a token without the subject_token purpose', () => { + const noPurpose = jwtService.sign({ sub: 'human-1' }); + + expect(() => service.validateSubjectAssertion(noPurpose)).toThrow('JWT Verification Failed'); + }); + + it('is not accepted as an access token by verifyOAuthAccessToken', async () => { + const token = service.mintSubjectAssertion('human-1'); + + const result = await service.verifyOAuthAccessToken(token); + + expect(result.user).toBeNull(); + }); + }); + + describe('actor tokens (token exchange)', () => { + it('returns the sub of a valid minted access token whose row still exists', async () => { + const actorSub = 'actor-1'; + const { accessToken } = service.generateTokenPair(actorSub, 'client-456', undefined, []); + accessTokenRepository.findOne.mockResolvedValue( + mock({ token: accessToken, clientId: 'client-456', userId: actorSub }), + ); + + await expect(service.validateActorToken(accessToken)).resolves.toBe(actorSub); + + // Identity proof only: audience is not enforced (no allowed-audiences lookup). + expect(accessTokenRepository.findOne).toHaveBeenCalledWith({ where: { token: accessToken } }); + }); + + it('rejects a tampered actor token', async () => { + const { accessToken } = service.generateTokenPair('actor-1', 'client-456', undefined, []); + + await expect(service.validateActorToken(`${accessToken}tampered`)).rejects.toThrow( + 'JWT Verification Failed', + ); + }); + + it('rejects an expired actor token', async () => { + const expired = jwtService.sign({ sub: 'actor-1', iat: 0, exp: 1 }); + + await expect(service.validateActorToken(expired)).rejects.toThrow('JWT Verification Failed'); + }); + + it('rejects an actor token whose row was deleted (revoked)', async () => { + const { accessToken } = service.generateTokenPair('actor-1', 'client-456', undefined, []); + accessTokenRepository.findOne.mockResolvedValue(null); + + await expect(service.validateActorToken(accessToken)).rejects.toThrow( + 'Access Token Not Found in Database', + ); + }); + + it('rejects a subject assertion presented as an actor token (no access-token row)', async () => { + // A subject assertion is signed and unexpired but persists no + // oauth_access_tokens row, so it can never stand in as an actor. + const subjectAssertion = service.mintSubjectAssertion('human-1'); + accessTokenRepository.findOne.mockResolvedValue(null); + + await expect(service.validateActorToken(subjectAssertion)).rejects.toThrow( + 'Access Token Not Found in Database', + ); + }); + + it('rejects a signed token missing a sub claim whose row exists', async () => { + const noSub = jwtService.sign({ foo: 'bar' }); + accessTokenRepository.findOne.mockResolvedValue( + mock({ token: noSub, clientId: 'client-456', userId: 'x' }), + ); + + await expect(service.validateActorToken(noSub)).rejects.toThrow('JWT Verification Failed'); + }); }); describe('saveTokenPair', () => { @@ -484,6 +605,59 @@ describe('OAuthTokenService', () => { }); }); + describe('verifyOAuthAccessToken delegation (act claim)', () => { + it('resolves the actor and authorizes against the subject for a delegated token', async () => { + const subjectId = 'subject-1'; + const actorId = 'actor-1'; + const clientId = 'client-456'; + const { accessToken } = service.generateTokenPair( + subjectId, + clientId, + undefined, + [], + actorId, + ); + + accessTokenRepository.findOne.mockResolvedValue( + mock({ token: accessToken, clientId, userId: subjectId }), + ); + const subject = mock({ id: subjectId }); + const actor = mock({ id: actorId }); + userRepository.findOne.mockResolvedValueOnce(subject).mockResolvedValueOnce(actor); + + const result = await service.verifyOAuthAccessToken(accessToken); + + expect(result.user).toBe(subject); + expect(result.actor).toBe(actor); + // The subject is resolved first (and is the identity the resource authorizes); + // the actor is resolved second, for attribution only. + expect(userRepository.findOne).toHaveBeenNthCalledWith(1, { + where: { id: subjectId }, + relations: ['role'], + }); + expect(userRepository.findOne).toHaveBeenNthCalledWith(2, { + where: { id: actorId }, + relations: ['role'], + }); + }); + + it('leaves actor undefined for a plain client_credentials token', async () => { + const subjectId = 'sub-only'; + const clientId = 'client-456'; + const { accessToken } = service.generateTokenPair(subjectId, clientId, undefined, []); + + accessTokenRepository.findOne.mockResolvedValue( + mock({ token: accessToken, clientId, userId: subjectId }), + ); + userRepository.findOne.mockResolvedValue(mock({ id: subjectId })); + + const result = await service.verifyOAuthAccessToken(accessToken); + + expect(result.user).not.toBeNull(); + expect(result.actor).toBeUndefined(); + }); + }); + describe('verifyOAuthAccessToken audit events', () => { it('emits service-account-token-verified with success and the resolved subject on the happy path', async () => { const userId = 'user-123'; @@ -497,8 +671,40 @@ describe('OAuthTokenService', () => { await service.verifyOAuthAccessToken(accessToken, TEST_RESOURCE_URL); + // A plain (autonomous) token carries no actor. expect(eventService.emit).toHaveBeenCalledWith('service-account-token-verified', { sub: userId, + act: null, + aud: TEST_RESOURCE_URL, + outcome: 'success', + }); + }); + + it('emits service-account-token-verified with the resolved actor for a delegated token', async () => { + const subjectId = 'human-1'; + const actorId = 'actor-1'; + const clientId = 'client-456'; + const { accessToken } = service.generateTokenPair( + subjectId, + clientId, + TEST_RESOURCE_URL, + [], + actorId, + ); + + accessTokenRepository.findOne.mockResolvedValue( + mock({ token: accessToken, clientId, userId: subjectId }), + ); + userRepository.findOne + .mockResolvedValueOnce(mock({ id: subjectId })) + .mockResolvedValueOnce(mock({ id: actorId })); + + await service.verifyOAuthAccessToken(accessToken, TEST_RESOURCE_URL); + + // `sub` stays the subject; `act` carries the delegated actor from `act.sub`. + expect(eventService.emit).toHaveBeenCalledWith('service-account-token-verified', { + sub: subjectId, + act: actorId, aud: TEST_RESOURCE_URL, outcome: 'success', }); @@ -509,6 +715,7 @@ describe('OAuthTokenService', () => { expect(eventService.emit).toHaveBeenCalledWith('service-account-token-verified', { sub: null, + act: null, aud: TEST_RESOURCE_URL, outcome: 'failure', }); @@ -528,6 +735,7 @@ describe('OAuthTokenService', () => { expect(eventService.emit).toHaveBeenCalledWith('service-account-token-verified', { sub: userId, + act: null, aud: TEST_RESOURCE_URL, outcome: 'failure', }); diff --git a/packages/cli/src/modules/oauth-server/__tests__/oauth.controller.api.test.ts b/packages/cli/src/modules/oauth-server/__tests__/oauth.controller.api.test.ts index 0ac5a2a4ed9..faf9cdbd3a3 100644 --- a/packages/cli/src/modules/oauth-server/__tests__/oauth.controller.api.test.ts +++ b/packages/cli/src/modules/oauth-server/__tests__/oauth.controller.api.test.ts @@ -41,7 +41,12 @@ describe('GET /.well-known/oauth-authorization-server', () => { registration_endpoint: expect.stringContaining('/mcp-oauth/register'), revocation_endpoint: expect.stringContaining('/mcp-oauth/revoke'), response_types_supported: ['code'], - grant_types_supported: ['authorization_code', 'refresh_token'], + grant_types_supported: [ + 'authorization_code', + 'refresh_token', + 'client_credentials', + 'urn:ietf:params:oauth:grant-type:token-exchange', + ], token_endpoint_auth_methods_supported: ['none', 'client_secret_post', 'client_secret_basic'], code_challenge_methods_supported: ['S256'], authorization_response_iss_parameter_supported: true, @@ -529,6 +534,46 @@ describe('POST /mcp-oauth/token', () => { }); }); +describe('POST /mcp-oauth/token (token-exchange grant)', () => { + const TOKEN_EXCHANGE_GRANT_TYPE = 'urn:ietf:params:oauth:grant-type:token-exchange'; + + beforeEach(async () => { + await mcpSettingsService.setEnabled(true); + }); + + afterEach(async () => { + await mcpSettingsService.setEnabled(false); + }); + + test('rejects a token-exchange request missing actor_token', async () => { + const response = await testServer.restlessAgent.post('/mcp-oauth/token').send({ + grant_type: TOKEN_EXCHANGE_GRANT_TYPE, + subject_token: 'a-subject-assertion', + subject_token_type: 'urn:ietf:params:oauth:token-type:jwt', + resource: 'https://n8n.example.com/mcp-server/http', + }); + + expect(response.statusCode).toBe(400); + expect(response.body.error).toBe('invalid_request'); + expect(response.body.error_description).toContain('actor_token'); + }); + + test('rejects a token-exchange request with a mismatched actor_token_type', async () => { + const response = await testServer.restlessAgent.post('/mcp-oauth/token').send({ + grant_type: TOKEN_EXCHANGE_GRANT_TYPE, + subject_token: 'a-subject-assertion', + subject_token_type: 'urn:ietf:params:oauth:token-type:jwt', + actor_token: 'an-actor-access-token', + actor_token_type: 'urn:ietf:params:oauth:token-type:jwt', + resource: 'https://n8n.example.com/mcp-server/http', + }); + + expect(response.statusCode).toBe(400); + expect(response.body.error).toBe('invalid_request'); + expect(response.body.error_description).toContain('actor_token_type'); + }); +}); + describe('POST /mcp-oauth/revoke', () => { beforeEach(async () => { await mcpSettingsService.setEnabled(true); diff --git a/packages/cli/src/modules/oauth-server/internal-oauth2-mint.service.ts b/packages/cli/src/modules/oauth-server/internal-oauth2-mint.service.ts index 35fc71ccf6f..0c8663e599b 100644 --- a/packages/cli/src/modules/oauth-server/internal-oauth2-mint.service.ts +++ b/packages/cli/src/modules/oauth-server/internal-oauth2-mint.service.ts @@ -8,6 +8,8 @@ import { EventService } from '@/events/event.service'; import { ServiceAccountCredentialService } from '@/services/service-account-credential.service'; import { UrlService } from '@/services/url.service'; +import { OAuthTokenService } from './oauth-token.service'; + /** * Fallback root-level token endpoint (no `/rest` prefix), used only when the * target isn't an n8n-served protected resource and discovery can't find an @@ -44,6 +46,7 @@ export class InternalOAuth2MintService { private readonly urlService: UrlService, private readonly eventService: EventService, private readonly logger: Logger, + private readonly oauthTokenService: OAuthTokenService, outboundHttp: OutboundHttp, ) { this.http = outboundHttp.requests({ @@ -53,20 +56,30 @@ export class InternalOAuth2MintService { } /** - * Mint a bearer access token for `userId`, audience-locked to `targetUrl`. + * Mint a bearer access token for `userId` (the acting service account), + * audience-locked to `targetUrl`. + * + * When `onBehalfOfUserId` is set — an interactive run a human triggered — the + * mint is *delegated* and runs in two token-endpoint calls: (1) obtain the + * service account's OWN access token via the `client_credentials` grant, then + * (2) present that token as the RFC 8693 `actor_token` in a token-exchange + * request alongside a subject assertion for the human, yielding a token whose + * `sub` is the human and whose `act` is the acting service account. Omitting it + * keeps the single-call autonomous `client_credentials` path unchanged. * * @throws OperationalError when the user has no service-account credential, or when - * the token endpoint call fails (network error or non-2xx). The client secret is - * never included in the thrown error. + * a token endpoint call fails (network error or non-2xx). The client secret, + * minted tokens and subject assertion are never included in the thrown error. */ async mintForUser( userId: string, targetUrl: string, ctx: OperationContext = {}, + onBehalfOfUserId?: string, ): Promise { const credential = await this.serviceAccountCredentialService.getDecryptedForUser(userId, ctx); if (!credential) { - this.emitOutcome(userId, '', targetUrl, 'failure'); + this.emitOutcome(userId, '', targetUrl, 'failure', onBehalfOfUserId); throw new OperationalError('No service-account credential for identity'); } @@ -100,14 +113,95 @@ export class InternalOAuth2MintService { resource = targetUrl; } - const body = new URLSearchParams({ - grant_type: 'client_credentials', - client_id: clientId, - client_secret: clientSecret, - resource, - }); - // TODO: external AS support — flip OutboundHttp ssrf off-'disabled' to the egress filter + validate the issuer against the credential's AS binding before sending the secret. Internal-only today. + let accessToken: string; + try { + // Step 1 (both paths): the service account's OWN access token via the + // `client_credentials` grant. Autonomously this IS the minted token. + const clientCredentialsToken = await this.mintClientCredentials( + clientId, + clientSecret, + tokenEndpoint, + resource, + ); + + if (onBehalfOfUserId) { + // Step 2 (delegated): present the SA's own token as the RFC 8693 + // `actor_token` alongside a subject assertion vouching for the human, so + // the exchanged token's `sub` is the human and its `act` is the acting + // service account. The SA's client credentials authenticate the caller. + const subjectToken = this.oauthTokenService.mintSubjectAssertion(onBehalfOfUserId); + accessToken = await this.requestAccessToken( + tokenEndpoint, + new URLSearchParams({ + grant_type: 'urn:ietf:params:oauth:grant-type:token-exchange', + client_id: clientId, + client_secret: clientSecret, + subject_token: subjectToken, + subject_token_type: 'urn:ietf:params:oauth:token-type:jwt', + actor_token: clientCredentialsToken, + actor_token_type: 'urn:ietf:params:oauth:token-type:access_token', + resource, + }), + ); + } else { + accessToken = clientCredentialsToken; + } + } catch (error) { + this.emitOutcome(userId, clientId, resource, 'failure', onBehalfOfUserId); + throw error; + } + + // Trace: the acting identity self-authenticated and now holds a token for `resource`. + this.logger.info('Service account self-authenticated (internal OAuth2 mint)', { + userId, + clientId, + resource, + tokenEndpoint, + }); + if (onBehalfOfUserId) { + // Trace: the token is delegated — it acts for the human, as the service account. + this.logger.info('Minted delegated (on-behalf-of) service-account token', { + sub: onBehalfOfUserId, + act: userId, + resource, + }); + } + this.emitOutcome(userId, clientId, resource, 'success', onBehalfOfUserId); + return accessToken; + } + + /** + * Obtain the service account's OWN access token via the `client_credentials` + * grant. Autonomously this is the token we return; for a delegated mint it + * becomes the RFC 8693 `actor_token`. + */ + private async mintClientCredentials( + clientId: string, + clientSecret: string, + tokenEndpoint: string, + resource: string, + ): Promise { + return await this.requestAccessToken( + tokenEndpoint, + new URLSearchParams({ + grant_type: 'client_credentials', + client_id: clientId, + client_secret: clientSecret, + resource, + }), + ); + } + + /** + * POST an OAuth2 grant `body` to `tokenEndpoint` and return its `access_token`. + * Shared by the `client_credentials` and token-exchange grants. On a network + * error or non-2xx only the HTTP status is surfaced — never the request body, + * which carries the client secret and (for a delegated mint) the actor token. + * + * @throws OperationalError when the call fails or no `access_token` is returned. + */ + private async requestAccessToken(tokenEndpoint: string, body: URLSearchParams): Promise { let accessToken: string | undefined; try { const response = await this.http.request<{ access_token?: string }>({ @@ -118,26 +212,15 @@ export class InternalOAuth2MintService { }); accessToken = response?.access_token; } catch (error) { - // Only surface the HTTP status — never the request body, which carries the secret. const status = isHttpRequestError(error) ? error.response?.status : undefined; - this.logger.warn('Failed to mint service-account token', { userId, targetUrl, status }); - this.emitOutcome(userId, clientId, resource, 'failure'); + this.logger.warn('Failed to mint service-account token', { tokenEndpoint, status }); throw new OperationalError('Failed to mint service-account token'); } if (!accessToken) { - this.emitOutcome(userId, clientId, resource, 'failure'); throw new OperationalError('Token endpoint returned no access_token'); } - // Trace: the acting identity self-authenticated and now holds a token for `resource`. - this.logger.info('Service account self-authenticated (internal OAuth2 mint)', { - userId, - clientId, - resource, - tokenEndpoint, - }); - this.emitOutcome(userId, clientId, resource, 'success'); return accessToken; } @@ -206,9 +289,13 @@ export class InternalOAuth2MintService { clientId: string, targetUrl: string, outcome: 'success' | 'failure', + onBehalfOfUserId?: string, ): void { + // Autonomous: `sub` is the acting service account, no actor. Delegated: the + // human on whose behalf we mint is the `sub`, the acting SA is the `act`. this.eventService.emit('service-account-token-minted', { - sub: userId, + sub: onBehalfOfUserId ?? userId, + ...(onBehalfOfUserId ? { act: userId } : {}), clientId, aud: targetUrl, outcome, diff --git a/packages/cli/src/modules/oauth-server/oauth-server.service.ts b/packages/cli/src/modules/oauth-server/oauth-server.service.ts index c1c9dcb8082..0946ac04533 100644 --- a/packages/cli/src/modules/oauth-server/oauth-server.service.ts +++ b/packages/cli/src/modules/oauth-server/oauth-server.service.ts @@ -463,6 +463,106 @@ export class OAuthServerService implements OAuthServerProvider { }; } + /** + * RFC 8693 token exchange (internal on-behalf-of / delegation). + * + * Client authentication (`client_id`/`client_secret`) authenticates the *caller* + * of the endpoint. The RFC 8693 *actor* is derived separately from `actorToken` + * — a security token representing the acting party — decoupled from the client, + * so an actor need not be a client-credentials holder. We mint a *delegated* + * access token whose `sub` is the human named by the subject assertion and whose + * `act.sub` is the actor_token's subject, audience-locked to `resource`. + * Authorization at verify time keys on the subject; the actor is attribution only. + */ + async exchangeToken( + clientId: string, + clientSecret: string, + subjectToken: string, + actorToken: string, + resource: string, + ): Promise { + const decrypted = await this.serviceAccountCredentialService.getDecryptedByClientId(clientId); + + if (!decrypted) { + // Keep the precise reason (a mild client-enumeration oracle) at debug; ops + // still see a generic warn without the oracle. + this.logger.debug('token-exchange rejected', { clientId, reason: 'unknown_client' }); + this.logger.warn('token-exchange rejected', { clientId, reason: 'invalid_client' }); + this.emitTokenExchanged(null, null, clientId, resource, 'failure'); + return null; + } + + const { clientSecret: storedSecret } = decrypted; + + if (!timingSafeEqualStrings(clientSecret, storedSecret)) { + this.logger.debug('token-exchange rejected', { clientId, reason: 'bad_secret' }); + this.logger.warn('token-exchange rejected', { clientId, reason: 'invalid_client' }); + this.emitTokenExchanged(null, null, clientId, resource, 'failure'); + return null; + } + + // `sub`/`act` are surfaced to the failure audit below as they become known: + // the subject first, then the actor. Both null until validated. + let subjectUserId: string | null = null; + let actorUserId: string | null = null; + try { + // The subject assertion the AS itself signed identifies the human on whose + // behalf we act. Throws (rejecting the exchange) on a bad/expired assertion. + subjectUserId = this.tokenService.validateSubjectAssertion(subjectToken); + + // The actor is derived from the actor_token (RFC 8693), NOT from the + // authenticating client — this is what lets a non-service-account actor work. + // Throws (rejecting the exchange) on a bad/expired/revoked actor token. + actorUserId = await this.tokenService.validateActorToken(actorToken); + } catch (error) { + this.emitTokenExchanged(subjectUserId, actorUserId, clientId, resource, 'failure'); + throw error; + } + + // TODO: policy — whether this client may present this actor_token, and RFC 8693 + // may_act (whether the actor may act for the subject). Always-allowed for the + // internal PoC. + + // oauth_access_tokens FKs clientId -> oauth_clients (see exchangeClientCredentials). + await this.ensureServiceAccountClient(clientId); + + const { accessToken, jti } = await this.tokenService.generateAccessTokenOnly( + subjectUserId, + clientId, + resource, + [], + actorUserId, + ); + + // Trace: the delegation edge (subject on-behalf-of, actor service account) and + // the audience. `jti` correlates this mint to its later verify. Never the token/secret. + this.logger.info('Minted delegated (on-behalf-of) token', { + sub: subjectUserId, + act: actorUserId, + clientId, + resource, + jti, + }); + + this.emitTokenExchanged(subjectUserId, actorUserId, clientId, resource, 'success'); + + return { + access_token: accessToken, + token_type: 'Bearer', + expires_in: this.tokenService.getAccessTokenExpirySeconds(), + }; + } + + private emitTokenExchanged( + sub: string | null, + act: string | null, + clientId: string | null, + aud: string, + outcome: 'success' | 'failure', + ): void { + this.eventService.emit('service-account-token-exchanged', { sub, act, clientId, aud, outcome }); + } + /** Idempotently create the oauth_clients row backing a service-account client. */ private async ensureServiceAccountClient(clientId: string): Promise { const existing = await this.oauthClientRepository.findOneBy({ id: clientId }); diff --git a/packages/cli/src/modules/oauth-server/oauth-token.service.ts b/packages/cli/src/modules/oauth-server/oauth-token.service.ts index a5f552bc9a0..e9e26c779fc 100644 --- a/packages/cli/src/modules/oauth-server/oauth-token.service.ts +++ b/packages/cli/src/modules/oauth-server/oauth-token.service.ts @@ -3,6 +3,7 @@ import { AuthInfo } from '@modelcontextprotocol/sdk/server/auth/types.js'; import { OAuthTokens } from '@modelcontextprotocol/sdk/shared/auth.js'; import { Logger } from '@n8n/backend-common'; import { Time } from '@n8n/constants'; +import type { User } from '@n8n/db'; import { TransactionRunner, UserRepository } from '@n8n/db'; import { Service } from '@n8n/di'; import { ensureError } from '@n8n/utils/errors/ensure-error'; @@ -34,6 +35,9 @@ import { AccessTokenNotFoundError, JWTVerificationError } from './oauth.errors'; export class OAuthTokenService implements OAuthTokenVerifier { private readonly ACCESS_TOKEN_EXPIRY_SECONDS = 1 * Time.hours.toSeconds; private readonly REFRESH_TOKEN_EXPIRY_MS = 30 * Time.days.toMilliseconds; + // A subject assertion is a short-lived, single-use bridge consumed synchronously + // by the token-exchange grant; it never travels further than the AS itself. + private readonly SUBJECT_ASSERTION_EXPIRY_SECONDS = 60; constructor( private readonly logger: Logger, @@ -50,13 +54,96 @@ export class OAuthTokenService implements OAuthTokenVerifier { return this.ACCESS_TOKEN_EXPIRY_SECONDS; } + /** + * Mint a short-lived subject assertion for the internal token-exchange + * (on-behalf-of) grant. It represents the human "subject" the AS itself + * vouches for; the exchange grant validates it and mints a delegated access + * token whose `sub` is this user. + * + * It is NOT an access token: no `oauth_access_tokens` row is persisted and no + * `meta.isOAuth` marker is set, and the `purpose` claim keeps it from ever + * being replayed as one (see {@link validateSubjectAssertion}). + */ + mintSubjectAssertion(userId: string): string { + const now = Math.floor(Date.now() / 1000); + return this.jwtService.sign({ + sub: userId, + purpose: 'subject_token', + iat: now, + exp: now + this.SUBJECT_ASSERTION_EXPIRY_SECONDS, + }); + } + + /** + * Validate a subject assertion minted by {@link mintSubjectAssertion}: + * signature + expiry (enforced by `jwtService.verify`) and + * `purpose === 'subject_token'`. Returns the subject `sub`; throws otherwise. + */ + validateSubjectAssertion(token: string): string { + let decoded: unknown; + try { + decoded = this.jwtService.verify(token); + } catch { + throw new JWTVerificationError(); + } + + const purpose = this.getStringClaim(decoded, 'purpose'); + const sub = this.getStringClaim(decoded, 'sub'); + if (purpose !== 'subject_token' || !sub) { + throw new JWTVerificationError(); + } + + return sub; + } + + /** + * Validate an `actor_token` for the RFC 8693 token-exchange grant. Unlike a + * subject assertion, this is one of our OWN minted access tokens presented as + * an *identity proof* of the acting party: verify its signature/expiry, confirm + * its `oauth_access_tokens` row still exists (so a revoked token can't stand in + * as an actor — the same existence check {@link verifyTokenWithAudiences} uses), + * and return its `sub`. Throws otherwise. + * + * Audience is deliberately NOT enforced: an actor_token proves who is acting, + * it is not an access grant for a specific resource. + */ + async validateActorToken(token: string): Promise { + let decoded: unknown; + try { + decoded = this.jwtService.verify(token); + } catch { + throw new JWTVerificationError(); + } + + const accessTokenRecord = await this.accessTokenRepository.findOne({ + where: { token }, + }); + if (!accessTokenRecord) { + throw new AccessTokenNotFoundError(); + } + + const sub = this.getStringClaim(decoded, 'sub'); + if (!sub) { + throw new JWTVerificationError(); + } + + return sub; + } + async generateAccessTokenOnly( userId: string, clientId: string, resource: string | undefined, scopes: string[], + actorUserId?: string, ): Promise<{ accessToken: string; jti: string }> { - const { accessToken, jti } = this.generateTokenPair(userId, clientId, resource, scopes); + const { accessToken, jti } = this.generateTokenPair( + userId, + clientId, + resource, + scopes, + actorUserId, + ); // Persist the access token: verification checks the token row exists, so an // unsaved token would be rejected. No refresh token is issued for // client_credentials (RFC 6749 §4.4.3). @@ -71,6 +158,7 @@ export class OAuthTokenService implements OAuthTokenVerifier { clientId: string, resource: string | undefined, scopes: string[], + actorUserId?: string, ): { accessToken: string; refreshToken: string; jti: string } { // Pre-RFC-8707 clients omit the resource indicator; fall back to the // registry's default resource (the instance MCP server). @@ -87,6 +175,9 @@ export class OAuthTokenService implements OAuthTokenVerifier { const accessToken = this.jwtService.sign({ sub: userId, + // RFC 8693 `act` claim: the delegated (on-behalf-of) actor. Present only + // for token-exchange grants; autonomous client_credentials pass nothing. + ...(actorUserId ? { act: { sub: actorUserId } } : {}), aud: audience, client_id: clientId, jti, @@ -234,6 +325,10 @@ export class OAuthTokenService implements OAuthTokenVerifier { // `jti` is carried through `extra` purely as a log-correlation handle (mint↔verify). const jti = this.getStringClaim(decoded, 'jti'); + // RFC 8693 `act.sub`: the delegated actor (agent SA) on an on-behalf-of token. + // Absent on autonomous (client_credentials) tokens. + const actorId = this.getActorSub(decoded); + return { token, clientId, @@ -241,6 +336,7 @@ export class OAuthTokenService implements OAuthTokenVerifier { extra: { userId, ...(jti ? { jti } : {}), + ...(actorId ? { actorId } : {}), }, }; } @@ -266,8 +362,14 @@ export class OAuthTokenService implements OAuthTokenVerifier { sub: string | null, aud: string | undefined, outcome: 'success' | 'failure', + act: string | null = null, ) { - this.eventService.emit('service-account-token-verified', { sub, aud: aud ?? '', outcome }); + this.eventService.emit('service-account-token-verified', { + sub, + act, + aud: aud ?? '', + outcome, + }); } async verifyOAuthAccessToken(token: string, expectedAudience?: string): Promise { @@ -328,6 +430,11 @@ export class OAuthTokenService implements OAuthTokenVerifier { return { user: null, context: { reason: 'insufficient_scope', auth_type: 'oauth' } }; } + // On a delegated (on-behalf-of) token, resolve the actor (agent SA) + // alongside the subject. Authorization stays keyed on the SUBJECT above — + // the actor is surfaced for attribution only, never authorized against. + const { actorId, actor } = await this.resolveActor(authInfo); + // Trace: who is acting (verified user + client) against which resource, now. // `jti` correlates this verify back to the mint that issued the token. const jti = @@ -340,9 +447,10 @@ export class OAuthTokenService implements OAuthTokenVerifier { aud: expectedAudience, scope: authInfo.scopes.join(' '), ...(jti ? { jti } : {}), + ...(actorId ? { act: actorId } : {}), }); - this.emitTokenVerified(userId, expectedAudience, 'success'); - return { user, authType: 'oauth', scopes: authInfo.scopes }; + this.emitTokenVerified(userId, expectedAudience, 'success', actorId); + return { user, authType: 'oauth', scopes: authInfo.scopes, ...(actor ? { actor } : {}) }; } catch (error) { this.emitTokenVerified(null, expectedAudience, 'failure'); const errorForSure = ensureError(error); @@ -482,4 +590,34 @@ export class OAuthTokenService implements OAuthTokenVerifier { const claimValue = (payload as Record)[claim]; return typeof claimValue === 'string' ? claimValue : null; } + + /** Extract the RFC 8693 `act.sub` (delegated actor), or null when absent. */ + private getActorSub(payload: unknown): string | null { + if (!payload || typeof payload !== 'object') return null; + const act = (payload as Record).act; + return this.getStringClaim(act, 'sub'); + } + + /** + * Resolve the delegated actor (agent SA) carried on an on-behalf-of token via + * `authInfo.extra.actorId`. Returns the raw `actorId` (for trace logging) and + * the resolved `User` when it exists. Both are absent on autonomous tokens. + */ + private async resolveActor( + authInfo: AuthInfo, + ): Promise<{ actorId: string | null; actor?: User }> { + const actorId = + authInfo.extra && typeof authInfo.extra === 'object' + ? this.getStringClaim(authInfo.extra, 'actorId') + : null; + if (!actorId) { + return { actorId: null }; + } + const actor = + (await this.userRepository.findOne({ + where: { id: actorId }, + relations: ['role'], + })) ?? undefined; + return { actorId, actor }; + } } diff --git a/packages/cli/src/modules/oauth-server/oauth.controller.ts b/packages/cli/src/modules/oauth-server/oauth.controller.ts index 6312375d7f0..e5c449e1adf 100644 --- a/packages/cli/src/modules/oauth-server/oauth.controller.ts +++ b/packages/cli/src/modules/oauth-server/oauth.controller.ts @@ -77,6 +77,12 @@ const rfc9207IssuerParam: RequestHandler = (_req, res, next) => { next(); }; +const CLIENT_CREDENTIALS_GRANT_TYPE = 'client_credentials'; +const TOKEN_EXCHANGE_GRANT_TYPE = 'urn:ietf:params:oauth:grant-type:token-exchange'; +const SUBJECT_TOKEN_TYPE_JWT = 'urn:ietf:params:oauth:token-type:jwt'; +const ACTOR_TOKEN_TYPE_ACCESS = 'urn:ietf:params:oauth:token-type:access_token'; +const ISSUED_TOKEN_TYPE_ACCESS = 'urn:ietf:params:oauth:token-type:access_token'; + const ClientCredentialsGrantSchema = z.object({ grant_type: z.string(), client_id: z.string().optional(), @@ -84,9 +90,51 @@ const ClientCredentialsGrantSchema = z.object({ resource: z.string().optional(), }); +const TokenExchangeGrantSchema = z.object({ + grant_type: z.string(), + client_id: z.string().optional(), + client_secret: z.string().optional(), + subject_token: z.string().optional(), + subject_token_type: z.string().optional(), + actor_token: z.string().optional(), + actor_token_type: z.string().optional(), + resource: z.string().optional(), +}); + +/** + * Extract client credentials from the request body, falling back to HTTP Basic. + * Shared by the client_credentials and token-exchange grant guards so both + * accept the same client-authentication shapes. + */ +function extractClientAuth( + req: Request, + bodyClientId?: string, + bodyClientSecret?: string, +): { clientId?: string; clientSecret?: string } { + let clientId = bodyClientId; + let clientSecret = bodyClientSecret; + + if (!clientId || !clientSecret) { + const authorizationHeader = req.headers['authorization']; + if (authorizationHeader?.startsWith('Basic ')) { + const credentials = Buffer.from( + authorizationHeader.slice('Basic '.length), + 'base64', + ).toString('utf-8'); + const [authClientId, authClientSecret] = credentials.split(':'); + if (authClientId && authClientSecret) { + clientId = authClientId; + clientSecret = authClientSecret; + } + } + } + + return { clientId, clientSecret }; +} + const clientCredentialsExchangeGuard: RequestHandler = async (req, res, next) => { const parseResult = ClientCredentialsGrantSchema.safeParse(req.body); - if (!parseResult.success || parseResult.data.grant_type !== 'client_credentials') { + if (!parseResult.success || parseResult.data.grant_type !== CLIENT_CREDENTIALS_GRANT_TYPE) { // Not our grant — hand off to the SDK token handler. next(); return; @@ -101,24 +149,12 @@ const clientCredentialsExchangeGuard: RequestHandler = async (req, res, next) => }); return; } - let clientId = parseResult.data.client_id; - let clientSecret = parseResult.data.client_secret; const resource = parseResult.data.resource; - - if (!clientId || !clientSecret) { - const authorizationHeader = req.headers['authorization']; - if (authorizationHeader?.startsWith('Basic ')) { - const credentials = Buffer.from( - authorizationHeader.slice('Basic '.length), - 'base64', - ).toString('utf-8'); - const [authClientId, authClientSecret] = credentials.split(':'); - if (authClientId && authClientSecret) { - clientId = authClientId; - clientSecret = authClientSecret; - } - } - } + const { clientId, clientSecret } = extractClientAuth( + req, + parseResult.data.client_id, + parseResult.data.client_secret, + ); if (!clientId || !clientSecret) { res.status(400).json({ @@ -150,6 +186,111 @@ const clientCredentialsExchangeGuard: RequestHandler = async (req, res, next) => } }; +/** + * RFC 8693 token-exchange grant (internal on-behalf-of delegation). Sibling to + * `clientCredentialsExchangeGuard`: parses the body, and on a non-match hands + * off to the SDK token handler via `next()`. Client authentication uses the + * same extraction as the client_credentials grant. + */ +const tokenExchangeGuard: RequestHandler = async (req, res, next) => { + const parseResult = TokenExchangeGrantSchema.safeParse(req.body); + if (!parseResult.success || parseResult.data.grant_type !== TOKEN_EXCHANGE_GRANT_TYPE) { + // Not our grant — hand off to the SDK token handler. + next(); + return; + } + + res.setHeader('Cache-Control', 'no-store'); + try { + const { + resource, + subject_token: subjectToken, + subject_token_type: subjectTokenType, + actor_token: actorToken, + actor_token_type: actorTokenType, + } = parseResult.data; + + if (!resource) { + res.status(400).json({ + error: 'invalid_request', + error_description: 'Missing required parameter: resource', + }); + return; + } + + if (!subjectToken) { + res.status(400).json({ + error: 'invalid_request', + error_description: 'Missing required parameter: subject_token', + }); + return; + } + + // Only the AS-issued JWT subject-token type is accepted. Absent is tolerated + // (RFC 8693 makes it OPTIONAL), a present mismatch is rejected. + if (subjectTokenType && subjectTokenType !== SUBJECT_TOKEN_TYPE_JWT) { + res.status(400).json({ + error: 'invalid_request', + error_description: `Unsupported subject_token_type: ${subjectTokenType}`, + }); + return; + } + + if (!actorToken) { + res.status(400).json({ + error: 'invalid_request', + error_description: 'Missing required parameter: actor_token', + }); + return; + } + + // The actor_token is one of our own minted access tokens. Absent type is + // tolerated (RFC 8693 makes it OPTIONAL), a present mismatch is rejected. + if (actorTokenType && actorTokenType !== ACTOR_TOKEN_TYPE_ACCESS) { + res.status(400).json({ + error: 'invalid_request', + error_description: `Unsupported actor_token_type: ${actorTokenType}`, + }); + return; + } + + const { clientId, clientSecret } = extractClientAuth( + req, + parseResult.data.client_id, + parseResult.data.client_secret, + ); + + if (!clientId || !clientSecret) { + res.status(400).json({ + error: 'invalid_request', + error_description: 'Missing required parameters: client_id and/or client_secret', + }); + return; + } + + // Trace: an inbound token-exchange (delegation) mint was requested. Never the secret/token. + logger.debug('token-exchange requested', { clientId, resource }); + + const response = await oauthServerService.exchangeToken( + clientId, + clientSecret, + subjectToken, + actorToken, + resource, + ); + if (!response) { + res.status(401).json({ error: 'invalid_client' }); + return; + } + res.status(200).json({ ...response, issued_token_type: ISSUED_TOKEN_TYPE_ACCESS }); + } catch (error) { + // We own the token-exchange grant here — surface failures directly instead of + // falling through to the SDK handler, which would mask them as `invalid_client`. + logger.error('Error handling token exchange', { error }); + res.status(500).json({ error: 'server_error' }); + } +}; + // Built once and mounted under both the legacy `/mcp-oauth/*` paths (existing // DCR clients hold them in their stored discovery metadata) and the neutral // `/oauth/*` paths that future, non-MCP protected resources will advertise. @@ -185,7 +326,7 @@ const sharedEndpointRouters = (basePath: '/mcp-oauth' | '/oauth'): StaticRouterM path: `${basePath}/token`, router: tokenRouter, skipAuth: true, - middlewares: [clientCredentialsExchangeGuard], + middlewares: [clientCredentialsExchangeGuard, tokenExchangeGuard], ipRateLimit: createIpRateLimit( oauthServerConfig.rateLimitToken, 5 * Time.minutes.toMilliseconds, @@ -263,7 +404,12 @@ export class OAuthController { registration_endpoint: `${baseUrl}/mcp-oauth/register`, revocation_endpoint: `${baseUrl}/mcp-oauth/revoke`, response_types_supported: ['code'], - grant_types_supported: ['authorization_code', 'refresh_token', 'client_credentials'], + grant_types_supported: [ + 'authorization_code', + 'refresh_token', + 'client_credentials', + TOKEN_EXCHANGE_GRANT_TYPE, + ], token_endpoint_auth_methods_supported: ['none', 'client_secret_post', 'client_secret_basic'], code_challenge_methods_supported: ['S256'], // RFC 9207: we include the `iss` parameter on authorization responses diff --git a/packages/cli/src/node-execution/__tests__/ephemeral-node-executor.test.ts b/packages/cli/src/node-execution/__tests__/ephemeral-node-executor.test.ts index 032d3a8211e..c8eabb36ef0 100644 --- a/packages/cli/src/node-execution/__tests__/ephemeral-node-executor.test.ts +++ b/packages/cli/src/node-execution/__tests__/ephemeral-node-executor.test.ts @@ -822,6 +822,7 @@ describe('EphemeralNodeExecutor', () => { describe('acting service-account identity (outbound mint hook)', () => { const runToolWith = async ( actingServiceAccountUserId?: string, + actingOnBehalfOfUserId?: string, ): Promise> => { const base: Record = {}; mockGetBase.mockResolvedValue(base); @@ -837,6 +838,7 @@ describe('EphemeralNodeExecutor', () => { inputData: [], projectId: 'p-1', actingServiceAccountUserId, + actingOnBehalfOfUserId, }); expect(result.status).toBe('success'); @@ -850,18 +852,44 @@ describe('EphemeralNodeExecutor', () => { const additionalData = await runToolWith('sa-user-1'); expect(additionalData.actingServiceAccountUserId).toBe('sa-user-1'); + expect(additionalData.actingOnBehalfOfUserId).toBeUndefined(); expect(typeof additionalData.mintInternalOAuth2Token).toBe('function'); const mint = additionalData.mintInternalOAuth2Token as (url: string) => Promise; await expect(mint('https://mcp.example.com/mcp')).resolves.toBe('minted-token'); - // The identity is server-set; the node never supplies it. - expect(mintForUser).toHaveBeenCalledWith('sa-user-1', 'https://mcp.example.com/mcp'); + // The identity is server-set; the node never supplies it. No human → autonomous mint. + expect(mintForUser).toHaveBeenCalledWith( + 'sa-user-1', + 'https://mcp.example.com/mcp', + {}, + undefined, + ); + }); + + it('threads the on-behalf-of human so the mint hook delegates', async () => { + const mintForUser = vi.fn().mockResolvedValue('delegated-token'); + Container.set(InternalOAuth2MintService, { mintForUser } as never); + + const additionalData = await runToolWith('sa-user-1', 'human-1'); + + expect(additionalData.actingServiceAccountUserId).toBe('sa-user-1'); + expect(additionalData.actingOnBehalfOfUserId).toBe('human-1'); + + const mint = additionalData.mintInternalOAuth2Token as (url: string) => Promise; + await expect(mint('https://mcp.example.com/mcp')).resolves.toBe('delegated-token'); + expect(mintForUser).toHaveBeenCalledWith( + 'sa-user-1', + 'https://mcp.example.com/mcp', + {}, + 'human-1', + ); }); it('leaves both unset when no acting identity is threaded (fails closed downstream)', async () => { const additionalData = await runToolWith(undefined); expect(additionalData.actingServiceAccountUserId).toBeUndefined(); + expect(additionalData.actingOnBehalfOfUserId).toBeUndefined(); expect(additionalData.mintInternalOAuth2Token).toBeUndefined(); }); }); diff --git a/packages/cli/src/node-execution/ephemeral-node-executor.ts b/packages/cli/src/node-execution/ephemeral-node-executor.ts index 0729da73947..55950e6ba13 100644 --- a/packages/cli/src/node-execution/ephemeral-node-executor.ts +++ b/packages/cli/src/node-execution/ephemeral-node-executor.ts @@ -51,6 +51,12 @@ export type EphemeralWorkflowToolLike = { * only — never derived from node input. */ actingServiceAccountUserId?: string; + /** + * Human on whose behalf an interactive run acts. When set alongside + * `actingServiceAccountUserId`, the ephemeral execution's mint hook is + * delegated (sub = human, act = SA). Server-set only. + */ + actingOnBehalfOfUserId?: string; /** Eval-only additionalData decoration (e.g. HTTP mock handler) — never set on production paths. */ configureAdditionalData?: (additionalData: IWorkflowExecuteAdditionalData) => void; }; @@ -74,6 +80,11 @@ export interface InlineNodeExecutionRequest { * {@link EphemeralWorkflowToolLike.actingServiceAccountUserId}). Server-set only. */ actingServiceAccountUserId?: string; + /** + * Human on whose behalf an interactive run acts (see + * {@link EphemeralWorkflowToolLike.actingOnBehalfOfUserId}). Server-set only. + */ + actingOnBehalfOfUserId?: string; /** Eval-only additionalData decoration (e.g. HTTP mock handler) — never set on production paths. */ configureAdditionalData?: (additionalData: IWorkflowExecuteAdditionalData) => void; } @@ -324,11 +335,17 @@ export class EphemeralNodeExecutor { // agent's service account. Built here (rather than threaded as a closure) so // the identity string is the only thing carried through the tool layers. const actingServiceAccountUserId = tool.actingServiceAccountUserId; + // Present only on human-triggered (interactive) runs; delegates the mint + // (sub = human, act = SA). Undefined on autonomous runs → autonomous mint. + const actingOnBehalfOfUserId = tool.actingOnBehalfOfUserId; additionalData.actingServiceAccountUserId = actingServiceAccountUserId; + additionalData.actingOnBehalfOfUserId = actingOnBehalfOfUserId; additionalData.mintInternalOAuth2Token = async (targetUrl: string) => await Container.get(InternalOAuth2MintService).mintForUser( actingServiceAccountUserId, targetUrl, + {}, + actingOnBehalfOfUserId, ); } tool.configureAdditionalData?.(additionalData); @@ -452,6 +469,7 @@ export class EphemeralNodeExecutor { credentials: mergedCredentials, nodeName: request.nodeName, actingServiceAccountUserId: request.actingServiceAccountUserId, + actingOnBehalfOfUserId: request.actingOnBehalfOfUserId, configureAdditionalData: request.configureAdditionalData, }; diff --git a/packages/frontend/@n8n/frontend-constants/src/views.ts b/packages/frontend/@n8n/frontend-constants/src/views.ts index 84754a3af87..5157284bcbb 100644 --- a/packages/frontend/@n8n/frontend-constants/src/views.ts +++ b/packages/frontend/@n8n/frontend-constants/src/views.ts @@ -57,6 +57,7 @@ export enum VIEWS { EVALUATION_COLLECTION_COMPARE = 'EvaluationCollectionCompare', USAGE = 'Usage', LOG_STREAMING_SETTINGS = 'LogStreamingSettingsView', + AUDIT_LOG_SETTINGS = 'AuditLogSettingsView', OPENTELEMETRY_SETTINGS = 'SettingsOpenTelemetryView', SSO_SETTINGS = 'SSoSettings', ENCRYPTION_KEYS_SETTINGS = 'EncryptionKeysSettings', diff --git a/packages/frontend/@n8n/i18n/src/locales/en.json b/packages/frontend/@n8n/i18n/src/locales/en.json index 3c5479772bc..dacc6131ae5 100644 --- a/packages/frontend/@n8n/i18n/src/locales/en.json +++ b/packages/frontend/@n8n/i18n/src/locales/en.json @@ -3741,6 +3741,12 @@ "settings.roles": "Roles", "settings.api": "API", "settings.n8napi": "n8n API", + "settings.auditLog.title": "Audit Log", + "settings.auditLog.description": "Debug view of events captured by the audit-log database sink.", + "settings.auditLog.filter.placeholder": "Filter by event name prefix, e.g. n8n.audit", + "settings.auditLog.column.ts": "Timestamp", + "settings.auditLog.column.eventName": "Event", + "settings.auditLog.column.payload": "Payload", "settings.log-streaming": "Log Streaming", "settings.log-streaming.heading": "Log Streaming", "settings.log-streaming.add": "Add new destination", diff --git a/packages/frontend/@n8n/rest-api-client/src/api/audit-log.ts b/packages/frontend/@n8n/rest-api-client/src/api/audit-log.ts new file mode 100644 index 00000000000..d7b7ccaeb5f --- /dev/null +++ b/packages/frontend/@n8n/rest-api-client/src/api/audit-log.ts @@ -0,0 +1,22 @@ +import type { IRestApiContext } from '../types'; +import { makeRestApiRequest } from '../utils'; + +export interface AuditLogEventRecord { + id: string; + eventName: string; + message: string; + ts: string; + payload: unknown; +} + +export interface AuditLogEventList { + data: AuditLogEventRecord[]; + count: number; +} + +export async function getAuditLogEvents( + context: IRestApiContext, + options: { skip?: number; take?: number; prefix?: string } = {}, +): Promise { + return await makeRestApiRequest(context, 'GET', '/eventbus/audit-log-events', { ...options }); +} diff --git a/packages/frontend/@n8n/rest-api-client/src/api/index.ts b/packages/frontend/@n8n/rest-api-client/src/api/index.ts index 8dc81622d82..559b2e151fa 100644 --- a/packages/frontend/@n8n/rest-api-client/src/api/index.ts +++ b/packages/frontend/@n8n/rest-api-client/src/api/index.ts @@ -1,5 +1,6 @@ export * from './ai-usage'; export * from './api-keys'; +export * from './audit-log'; export * from './cloudPlans'; export * from './communityNodes'; export * from './credentialResolvers'; diff --git a/packages/frontend/editor-ui/src/app/composables/useSettingsItems.ts b/packages/frontend/editor-ui/src/app/composables/useSettingsItems.ts index 13bebcdf7d0..2936a51854c 100644 --- a/packages/frontend/editor-ui/src/app/composables/useSettingsItems.ts +++ b/packages/frontend/editor-ui/src/app/composables/useSettingsItems.ts @@ -174,6 +174,15 @@ export function useSettingsItems() { route: { to: { name: VIEWS.LOG_STREAMING_SETTINGS } }, }); + menuItems.push({ + id: 'settings-audit-log', + icon: 'scroll-text', + label: i18n.baseText('settings.auditLog.title'), + position: 'top', + available: canUserAccessRouteByName(VIEWS.AUDIT_LOG_SETTINGS), + route: { to: { name: VIEWS.AUDIT_LOG_SETTINGS } }, + }); + menuItems.push({ id: 'settings-opentelemetry', icon: 'telescope', diff --git a/packages/frontend/editor-ui/src/app/router.ts b/packages/frontend/editor-ui/src/app/router.ts index 88cac16c790..b39e2255414 100644 --- a/packages/frontend/editor-ui/src/app/router.ts +++ b/packages/frontend/editor-ui/src/app/router.ts @@ -60,6 +60,8 @@ const SettingsApiView = async () => await import('@/features/settings/apiKeys/views/SettingsApiView.vue'); const SettingsLogStreamingView = async () => await import('@/features/integrations/logStreaming.ee/views/SettingsLogStreamingView.vue'); +const AuditLogView = async () => + await import('@/features/settings/auditLog/views/AuditLogView.vue'); const SetupView = async () => await import('@/features/core/auth/views/SetupView.vue'); const SigninView = async () => await import('@/features/core/auth/views/SigninView.vue'); const SignupView = async () => await import('@/features/core/auth/views/SignupView.vue'); @@ -1093,6 +1095,22 @@ export const routes: RouteRecordRaw[] = [ }, }, }, + { + path: 'audit-log', + name: VIEWS.AUDIT_LOG_SETTINGS, + component: AuditLogView, + meta: { + middleware: ['authenticated', 'rbac'], + middlewareOptions: { + rbac: { + scope: 'logStreaming:manage', + }, + }, + telemetry: { + pageCategory: 'settings', + }, + }, + }, { path: 'workers', name: VIEWS.WORKER_VIEW, diff --git a/packages/frontend/editor-ui/src/features/settings/auditLog/auditLog.store.ts b/packages/frontend/editor-ui/src/features/settings/auditLog/auditLog.store.ts new file mode 100644 index 00000000000..4978de22bef --- /dev/null +++ b/packages/frontend/editor-ui/src/features/settings/auditLog/auditLog.store.ts @@ -0,0 +1,49 @@ +import { defineStore } from 'pinia'; +import { ref } from 'vue'; +import { useRootStore } from '@n8n/stores/useRootStore'; +import * as auditLogApi from '@n8n/rest-api-client/api/audit-log'; +import type { AuditLogEventRecord } from '@n8n/rest-api-client/api/audit-log'; +import type { TableOptions } from '@n8n/design-system/components/N8nDataTableServer'; + +const DEFAULT_PAGE_SIZE = 25; +const initialTableOptions = (): TableOptions => ({ + page: 0, + itemsPerPage: DEFAULT_PAGE_SIZE, + sortBy: [], +}); + +// ponytail: local store, no STORES enum entry (PoC debug page). +export const useAuditLogStore = defineStore('auditLog', () => { + const rootStore = useRootStore(); + + const events = ref([]); + const count = ref(0); + const prefix = ref(''); + const tableOptions = ref(initialTableOptions()); + + const fetchEvents = async () => { + const opts = tableOptions.value; + const trimmed = prefix.value.trim(); + const response = await auditLogApi.getAuditLogEvents(rootStore.restApiContext, { + take: opts.itemsPerPage, + skip: Math.max(0, opts.page) * opts.itemsPerPage, + ...(trimmed ? { prefix: trimmed } : {}), + }); + events.value = response.data; + count.value = response.count; + }; + + const setPrefix = async (value: string) => { + if (prefix.value === value) return; + prefix.value = value; + tableOptions.value.page = 0; + await fetchEvents(); + }; + + // DTS already wrote the new page/itemsPerPage via v-model; just refetch. + const applyTableOptions = async () => { + await fetchEvents(); + }; + + return { events, count, prefix, tableOptions, fetchEvents, setPrefix, applyTableOptions }; +}); diff --git a/packages/frontend/editor-ui/src/features/settings/auditLog/views/AuditLogView.vue b/packages/frontend/editor-ui/src/features/settings/auditLog/views/AuditLogView.vue new file mode 100644 index 00000000000..17a56b06e2d --- /dev/null +++ b/packages/frontend/editor-ui/src/features/settings/auditLog/views/AuditLogView.vue @@ -0,0 +1,124 @@ + + + + + diff --git a/packages/workflow/src/interfaces.ts b/packages/workflow/src/interfaces.ts index 6f9884b607b..f07c54846b8 100644 --- a/packages/workflow/src/interfaces.ts +++ b/packages/workflow/src/interfaces.ts @@ -3654,6 +3654,14 @@ export interface IWorkflowExecuteAdditionalData { */ actingServiceAccountUserId?: string; mintInternalOAuth2Token?: (targetUrl: string) => Promise; + /** + * The human on whose behalf an interactive agent run acts. Set only when a + * human triggered the run (in-app chat/resume/task-now); absent on autonomous + * runs. When present it drives delegated (on-behalf-of) token minting — the + * mint hook issues a token whose `sub` is this user and whose `act` is the + * acting service account, instead of an autonomous service-account token. + */ + actingOnBehalfOfUserId?: string; currentNodeExecutionIndex: number; httpResponse?: express.Response; httpRequest?: express.Request; diff --git a/packages/workflow/src/message-event-bus.ts b/packages/workflow/src/message-event-bus.ts index c0be01d39a1..31947a0c7a3 100644 --- a/packages/workflow/src/message-event-bus.ts +++ b/packages/workflow/src/message-event-bus.ts @@ -25,6 +25,7 @@ export const enum MessageEventBusDestinationTypeNames { webhook = '$$MessageEventBusDestinationWebhook', sentry = '$$MessageEventBusDestinationSentry', syslog = '$$MessageEventBusDestinationSyslog', + database = '$$MessageEventBusDestinationDatabase', } export const messageEventBusDestinationTypeNames = [ @@ -32,6 +33,7 @@ export const messageEventBusDestinationTypeNames = [ MessageEventBusDestinationTypeNames.webhook, MessageEventBusDestinationTypeNames.sentry, MessageEventBusDestinationTypeNames.syslog, + MessageEventBusDestinationTypeNames.database, ]; // ===============================