From d9973483525b83e1569a897bb01a12936a9c8103 Mon Sep 17 00:00:00 2001 From: James Martin <393605+ymhr@users.noreply.github.com> Date: Wed, 2 Sep 2026 11:27:29 +0000 Subject: [PATCH] feat: Refresh chat OAuth2 access tokens periodically (no-changelog) (#37541) Co-authored-by: Claude Opus 5 (1M context) Co-authored-by: MHMD <45968454+dou-mhmd@users.noreply.github.com> --- .../trigger/ChatTrigger/ChatTrigger.node.ts | 26 +- .../trigger/ChatTrigger/GenericFunctions.ts | 147 +++++++-- .../__test__/ChatTrigger.node.test.ts | 52 ++- .../__test__/GenericFunctions.test.ts | 298 +++++++++++++++++- .../ChatTrigger/__test__/shell.test.ts | 160 +++++++++- .../ChatTrigger/__test__/templates.test.ts | 202 +++++++++++- .../nodes/trigger/ChatTrigger/shell.ts | 114 ++++++- .../nodes/trigger/ChatTrigger/templates.ts | 218 ++++++++++++- .../nodes/trigger/ChatTrigger/types.ts | 16 + packages/cli/src/constants.ts | 10 + .../__tests__/oauth-flow.service.api.test.ts | 126 ++++++++ .../__tests__/oauth-server.service.test.ts | 8 +- .../oauth-server/oauth-flow.service.ts | 64 +++- .../oauth-server/oauth-server.service.ts | 7 +- .../src/services/oauth2-flow-proxy.service.ts | 28 +- .../webhook-request-sanitizer.test.ts | 34 +- packages/cli/src/webhooks/webhook-helpers.ts | 3 + .../src/webhooks/webhook-request-sanitizer.ts | 13 +- .../node-execution-context/webhook-context.ts | 11 + .../nodes-base/nodes/Form/test/utils.test.ts | 4 + packages/workflow/src/interfaces.ts | 33 +- 21 files changed, 1489 insertions(+), 85 deletions(-) diff --git a/packages/@n8n/nodes-langchain/nodes/trigger/ChatTrigger/ChatTrigger.node.ts b/packages/@n8n/nodes-langchain/nodes/trigger/ChatTrigger/ChatTrigger.node.ts index bfbbb14f9d0..47e76ee618b 100644 --- a/packages/@n8n/nodes-langchain/nodes/trigger/ChatTrigger/ChatTrigger.node.ts +++ b/packages/@n8n/nodes-langchain/nodes/trigger/ChatTrigger/ChatTrigger.node.ts @@ -31,13 +31,16 @@ import { Container } from '@n8n/di'; import { cssVariables } from './constants'; import { establishChatSessionIdentity, + handleChatTokenRefresh, resolveInnerFrameIdentity, validateAuth, } from './GenericFunctions'; import { + buildChatRefreshUrl, buildInnerFrameSrc, CHAT_FRAME_SANDBOX, isChatOAuth2Enabled, + isChatRefreshRequest, isShellInnerRequest, } from './shell'; import { createPage, createShellPage } from './templates'; @@ -943,20 +946,37 @@ export class ChatTrigger extends Node { throw new NodeOperationError(ctx.getNode(), 'Default webhook url not set'); } + // The shell's token-refresh leg, ahead of any render: it answers with JSON, + // not a page, and authenticates itself from its own httpOnly cookie rather + // than from the handshake below. A GET because a POST to this path reaches + // the `default` webhook — the chat message endpoint — instead. + if (isChatRefreshRequest(req)) { + await handleChatTokenRefresh(ctx, resourceUrl); + return { noWebhookResponse: true }; + } + if (!isShellInnerRequest(req)) { // Outer shell: the AS handshake runs here — a normal top-level document with // real cookies, unlike the sandboxed, opaque-origin frame this shell is about // to create. It is the only gate: a visitor without an editor session is // authenticated by the flow rather than bounced to sign-in ahead of it. - const ready = await establishChatSessionIdentity(ctx, resourceUrl); - if (!ready) { + const session = await establishChatSessionIdentity(ctx, resourceUrl); + if (!session) { return { noWebhookResponse: true }; } res.setHeader('Content-Security-Policy', "frame-ancestors 'none'"); res .status(200) - .send(createShellPage({ iframeSrc: buildInnerFrameSrc(req) })) + .send( + createShellPage({ + iframeSrc: buildInnerFrameSrc(req), + refresh: { + url: buildChatRefreshUrl(req), + expiresIn: session.expiresIn, + }, + }), + ) .end(); return { noWebhookResponse: true }; } diff --git a/packages/@n8n/nodes-langchain/nodes/trigger/ChatTrigger/GenericFunctions.ts b/packages/@n8n/nodes-langchain/nodes/trigger/ChatTrigger/GenericFunctions.ts index 12fa9508651..3b2aff0b813 100644 --- a/packages/@n8n/nodes-langchain/nodes/trigger/ChatTrigger/GenericFunctions.ts +++ b/packages/@n8n/nodes-langchain/nodes/trigger/ChatTrigger/GenericFunctions.ts @@ -7,9 +7,25 @@ import { clearChatOAuthToken, isChatOAuth2Enabled, readChatOAuthToken, + readChatRefreshToken, setChatOAuthToken, + setChatRefreshToken, } from './shell'; -import type { AuthenticationChatOption, ChatFrameIdentity } from './types'; +import type { AuthenticationChatOption, ChatFrameIdentity, ChatShellSession } from './types'; + +/** Absolute expiry for an access token the AS just minted, from the duration it reported. */ +function expiryFrom(expiresIn: number): number { + return Date.now() + expiresIn * 1000; +} + +/** + * Seconds left on an absolute expiry, on the server's own clock. Converting here rather + * than in the page is the point: the page must never subtract its own `Date.now()` from + * a timestamp this process produced. + */ +function secondsUntil(expiresAt: number): number { + return Math.max(0, (expiresAt - Date.now()) / 1000); +} export async function validateAuth(context: IWebhookFunctions) { const authentication = context.getNodeParameter( @@ -102,16 +118,20 @@ export async function validateAuth(context: IWebhookFunctions) { * receive the AS's session-cookie check, and any consent/sign-in page the AS * falls back to would then render editor-ui inside the opaque frame. * - * On success, stashes the AS token in the one-hop `n8n-chat-oauth` cookie and - * returns `true` — the caller renders the shell, whose frame's own GET picks - * the cookie up via `resolveInnerFrameIdentity`. Returns `false` after - * already sending a redirect/error response — the caller must abort with - * `noWebhookResponse`. + * On success, stashes the AS token in the one-hop `n8n-chat-oauth` cookie, the + * grant's refresh token in the long-lived httpOnly `n8n-chat-oauth-refresh` + * cookie, and returns the session — the caller renders the shell around it, + * whose frame's own GET picks the one-hop cookie up via + * `resolveInnerFrameIdentity`. Returns `null` after already sending a + * redirect/error response — the caller must abort with `noWebhookResponse`. + * + * The returned session carries the expiry only. The refresh token stays in its + * cookie and never reaches the caller, so it can't reach a document either. */ export async function establishChatSessionIdentity( context: IWebhookFunctions, resourceUrl: string, -): Promise { +): Promise { const req = context.getRequestObject(); const res = context.getResponseObject(); const { code, state } = req.query; @@ -124,7 +144,7 @@ export async function establishChatSessionIdentity( }); res.status(403).send('Access denied'); res.end(); - return false; + return null; } if (typeof code === 'string' && typeof state === 'string') { @@ -134,11 +154,15 @@ export async function establishChatSessionIdentity( try { const result = await context.completeN8nOAuth2Flow(code, state); if (result.valid) { - setChatOAuthToken(res, req, resourceUrl, result.token); + setChatOAuthToken(res, req, resourceUrl, { + token: result.token, + expiresAt: expiryFrom(result.expiresIn), + }); + setChatRefreshToken(res, req, resourceUrl, result.refreshToken); const redirectPath = req.originalUrl.split('?')[0]; res.writeHead(302, { Location: redirectPath }); res.end(); - return false; + return null; } // Fall through to restart the OAuth2 flow if the callback is invalid. context.logger.warn('Chat OAuth2 flow failed, restarting', { reason: result.reason }); @@ -150,13 +174,20 @@ export async function establishChatSessionIdentity( // Not an AS callback. If we just completed the flow, the token rides in the // one-hop cookie set on the redirect above — leave it for the frame's own GET // to consume, just confirm it's still good before rendering the shell around it. - const cookieToken = readChatOAuthToken(req); - if (cookieToken) { - const validation = await context.validateN8nOAuth2Token(cookieToken, resourceUrl); + const session = readChatOAuthToken(req); + if (session) { + const validation = await context.validateN8nOAuth2Token(session.token, resourceUrl); if (validation.valid) { - return true; + return { expiresIn: secondsUntil(session.expiresAt) }; } // Stale/invalid cookie — fall through to restart the OAuth2 flow. + } else { + // A reload mid-conversation: the one-hop cookie is long gone, but the grant + // is still live in the refresh cookie. Rotating is cheaper than a full + // redirect round trip through the AS, and keeps the visitor on the page. + const refreshed = await refreshChatSession(context, resourceUrl); + if (refreshed) return { expiresIn: refreshed.expiresIn }; + // Refresh failed — fall through to restart the OAuth2 flow, which handles it. } } @@ -169,7 +200,83 @@ export async function establishChatSessionIdentity( context.logger.warn('Chat OAuth2 flow failed', { error }); throw new UnexpectedError('Chat OAuth2 flow failed'); } - return false; + return null; +} + +/** + * Rotate the grant behind the refresh cookie into a fresh pair and re-set both + * cookies. Returns the fresh access token and its lifetime, or `null` when there is + * no refresh cookie or the AS refuses it — the caller decides whether that means + * restart the flow or answer 401. + */ +async function refreshChatSession( + context: IWebhookFunctions, + resourceUrl: string, +): Promise<{ token: string; expiresIn: number } | null> { + const req = context.getRequestObject(); + const res = context.getResponseObject(); + + const refreshToken = readChatRefreshToken(req); + if (!refreshToken) return null; + + try { + const result = await context.refreshN8nOAuth2Flow(refreshToken, resourceUrl); + if (!result.valid) { + context.logger.warn('Chat OAuth2 refresh rejected', { reason: result.reason }); + return null; + } + const expiresAt = expiryFrom(result.expiresIn); + setChatOAuthToken(res, req, resourceUrl, { token: result.token, expiresAt }); + // Rotation invalidates the token we just sent, so the cookie must be replaced + // in the same response or the next refresh presents a consumed one. + setChatRefreshToken(res, req, resourceUrl, result.refreshToken); + return { token: result.token, expiresIn: result.expiresIn }; + } catch (error) { + context.logger.warn('Chat OAuth2 refresh failed', { error }); + return null; + } +} + +/** + * The shell's own refresh leg: a same-origin GET on the `setup` path that mints a + * fresh access token for the frame. Authenticates purely from the httpOnly refresh + * cookie — the shell's script never holds the refresh token and can't forge this. + * + * Answers the request itself; the caller must abort with `noWebhookResponse`. + */ +export async function handleChatTokenRefresh( + context: IWebhookFunctions, + resourceUrl: string, +): Promise { + const req = context.getRequestObject(); + const res = context.getResponseObject(); + + // `no-store` because the response body is a bearer token: a shared cache holding + // it would hand one visitor's token to the next. + res.setHeader('Cache-Control', 'no-store'); + + if (!readChatRefreshToken(req)) { + res.status(401).json({ error: 'invalid_grant' }); + res.end(); + return; + } + + const refreshed = await refreshChatSession(context, resourceUrl); + if (!refreshed) { + // The cookie stays. A concurrent refresh on the same path — a second tab — wins + // the AS's atomic rotation and has already written its rotated token here, so + // clearing would erase a live grant and take the winner down with the loser. A + // cookie the AS really has finished with self-heals instead: the next shell GET + // fails its refresh, redirects through the AS, and the callback overwrites it. + res.status(401).json({ error: 'invalid_grant' }); + res.end(); + return; + } + + // Only the access token crosses the wire; the rotated refresh token stays in its + // httpOnly cookie. `expiresIn` is a duration, so the page schedules off its own + // clock and never has to agree with the server's. + res.status(200).json({ token: refreshed.token, expiresIn: refreshed.expiresIn }).end(); } /** @@ -189,15 +296,17 @@ export async function resolveInnerFrameIdentity( const req = context.getRequestObject(); const res = context.getResponseObject(); - const cookieToken = readChatOAuthToken(req); - if (!cookieToken) { + const session = readChatOAuthToken(req); + if (!session) { return null; } + // Only the one-hop cookie: the refresh cookie has to survive this render, since + // every later refresh the shell asks for is authenticated by it. clearChatOAuthToken(res, req, resourceUrl); - const validation = await context.validateN8nOAuth2Token(cookieToken, resourceUrl); + const validation = await context.validateN8nOAuth2Token(session.token, resourceUrl); if (!validation.valid) { return null; } - return { visitor: validation.user, authToken: cookieToken }; + return { visitor: validation.user, authToken: session.token }; } diff --git a/packages/@n8n/nodes-langchain/nodes/trigger/ChatTrigger/__test__/ChatTrigger.node.test.ts b/packages/@n8n/nodes-langchain/nodes/trigger/ChatTrigger/__test__/ChatTrigger.node.test.ts index 4a84b6ec08e..3508a1f5899 100644 --- a/packages/@n8n/nodes-langchain/nodes/trigger/ChatTrigger/__test__/ChatTrigger.node.test.ts +++ b/packages/@n8n/nodes-langchain/nodes/trigger/ChatTrigger/__test__/ChatTrigger.node.test.ts @@ -8,6 +8,7 @@ import { ChatTrigger } from '../ChatTrigger.node'; import { ChatTriggerAuthorizationError } from '../error'; import { establishChatSessionIdentity, + handleChatTokenRefresh, resolveInnerFrameIdentity, validateAuth, } from '../GenericFunctions'; @@ -16,6 +17,7 @@ import type { LoadPreviousSessionChatOption } from '../types'; vi.mock('../GenericFunctions', () => ({ validateAuth: vi.fn(), establishChatSessionIdentity: vi.fn(), + handleChatTokenRefresh: vi.fn(), resolveInnerFrameIdentity: vi.fn(), })); @@ -571,7 +573,7 @@ describe('ChatTrigger Node', () => { typeVersion: 1.4, webhookId: 'webhook-1', } as never); - vi.mocked(establishChatSessionIdentity).mockResolvedValue(true); + vi.mocked(establishChatSessionIdentity).mockResolvedValue({ expiresIn: 3600 }); vi.mocked(resolveInnerFrameIdentity).mockResolvedValue({ visitor, authToken: 'as-token', @@ -631,7 +633,7 @@ describe('ChatTrigger Node', () => { // never on the sandboxed frame's own request — a redirect to sign-in/consent from // inside that opaque-origin frame would render editor-ui inside it and crash. it('does not render the shell while the outer AS handshake is still in flight', async () => { - vi.mocked(establishChatSessionIdentity).mockResolvedValue(false); + vi.mocked(establishChatSessionIdentity).mockResolvedValue(null); const result = await renderSetupPage(); @@ -698,6 +700,7 @@ describe('ChatTrigger Node', () => { expect(establishChatSessionIdentity).not.toHaveBeenCalled(); expect(renderedPage()).toContain('createChat'); expect(renderedPage()).not.toContain('n8nShellInner'); + expect(renderedPage()).not.toContain('n8nChatRefresh'); }); it.each(['none', 'basicAuth'])( @@ -709,9 +712,54 @@ describe('ChatTrigger Node', () => { expect(establishChatSessionIdentity).not.toHaveBeenCalled(); expect(renderedPage()).toContain('createChat'); expect(renderedPage()).not.toContain('n8nShellInner'); + expect(renderedPage()).not.toContain('n8nChatRefresh'); }, ); + it('carries the refresh endpoint and schedule into the shell', async () => { + await renderSetupPage(); + + expect(renderedPage()).toContain('/webhook/abc/chat?n8nChatRefresh=1'); + expect(renderedPage()).toContain("'x-n8n-chat-refresh': '1'"); + // The session's duration, passed straight through — no timestamp of ours + // for the page's clock to disagree with. + expect(renderedPage()).toContain('planFor(3600)'); + }); + + // The leg answers with JSON, not a page, and authenticates from its own httpOnly + // cookie — so it must be handled before either render branch decides anything. + it('routes the refresh leg ahead of the shell render', async () => { + mockRequest.query = { n8nChatRefresh: '1' }; + mockRequest.headers = { + 'x-forwarded-proto': 'http', + host: 'localhost:5678', + 'x-n8n-chat-refresh': '1', + 'sec-fetch-site': 'same-origin', + }; + + const result = await renderSetupPage(); + + expect(result).toEqual({ noWebhookResponse: true }); + expect(handleChatTokenRefresh).toHaveBeenCalledWith( + mockContext, + 'http://localhost:5678/webhook/abc/chat', + ); + expect(establishChatSessionIdentity).not.toHaveBeenCalled(); + expect(resolveInnerFrameIdentity).not.toHaveBeenCalled(); + expect(mockResponse.send).not.toHaveBeenCalled(); + }); + + // Without the custom header the request is forgeable by shape alone, so it must + // fall through to the ordinary shell render rather than reach the leg. + it('does not route the refresh leg without the custom header', async () => { + mockRequest.query = { n8nChatRefresh: '1' }; + + await renderSetupPage(); + + expect(handleChatTokenRefresh).not.toHaveBeenCalled(); + expect(establishChatSessionIdentity).toHaveBeenCalled(); + }); + // The builder opens the test URL from the canvas, so it must split exactly as // production does for the flow to be testable end to end. it('splits the page in test mode too', async () => { diff --git a/packages/@n8n/nodes-langchain/nodes/trigger/ChatTrigger/__test__/GenericFunctions.test.ts b/packages/@n8n/nodes-langchain/nodes/trigger/ChatTrigger/__test__/GenericFunctions.test.ts index 7eac95da1cf..fc42f67057a 100644 --- a/packages/@n8n/nodes-langchain/nodes/trigger/ChatTrigger/__test__/GenericFunctions.test.ts +++ b/packages/@n8n/nodes-langchain/nodes/trigger/ChatTrigger/__test__/GenericFunctions.test.ts @@ -4,6 +4,7 @@ import { mock } from 'vitest-mock-extended'; import { ChatTriggerAuthorizationError } from '../error'; import { establishChatSessionIdentity, + handleChatTokenRefresh, resolveInnerFrameIdentity, validateAuth, } from '../GenericFunctions'; @@ -295,14 +296,20 @@ describe('establishChatSessionIdentity', () => { const res = { status: vi.fn().mockReturnThis(), send: vi.fn().mockReturnThis(), + json: vi.fn().mockReturnThis(), end: vi.fn().mockReturnThis(), writeHead: vi.fn().mockReturnThis(), + setHeader: vi.fn().mockReturnThis(), cookie: vi.fn().mockReturnThis(), clearCookie: vi.fn().mockReturnThis(), }; return res as never; }; + /** The payload the one-hop cookie carries, percent-encoded as a browser sends it. */ + const oauthCookie = (token: string, expiresAt: number) => + `n8n-chat-oauth=${encodeURIComponent(JSON.stringify({ t: token, e: expiresAt }))}`; + beforeEach(() => { vi.clearAllMocks(); mockContext.getResponseObject.mockReturnValue(mockRes()); @@ -319,30 +326,43 @@ describe('establishChatSessionIdentity', () => { const result = await establishChatSessionIdentity(mockContext, resourceUrl); - expect(result).toBe(false); + expect(result).toBeNull(); expect(mockContext.beginN8nOAuth2Flow).toHaveBeenCalledWith(resourceUrl); expect(mockContext.getResponseObject().writeHead).toHaveBeenCalledWith(302, { Location: 'https://as.example.com/authorize', }); }); - it('completes the AS callback, hands the token off via a one-hop cookie, and redirects to the clean shell URL', async () => { + it('completes the AS callback, hands both cookies off, and redirects to the clean shell URL', async () => { mockContext.getRequestObject.mockReturnValue({ query: { code: 'auth-code', state: 'flow-state' }, headers: {}, originalUrl: '/webhook/abc/chat?code=auth-code&state=flow-state', } as never); - mockContext.completeN8nOAuth2Flow.mockResolvedValue({ valid: true, token: 'as-token', user }); + mockContext.completeN8nOAuth2Flow.mockResolvedValue({ + valid: true, + token: 'as-token', + refreshToken: 'refresh-token', + expiresIn: 3600, + user, + }); const result = await establishChatSessionIdentity(mockContext, resourceUrl); - expect(result).toBe(false); + expect(result).toBeNull(); expect(mockContext.completeN8nOAuth2Flow).toHaveBeenCalledWith('auth-code', 'flow-state'); expect(mockContext.getResponseObject().cookie).toHaveBeenCalledWith( 'n8n-chat-oauth', - 'as-token', + expect.stringContaining('as-token'), expect.objectContaining({ httpOnly: true }), ); + // The refresh token gets its own long-lived httpOnly cookie; nothing else + // carries it, so it never reaches a document. + expect(mockContext.getResponseObject().cookie).toHaveBeenCalledWith( + 'n8n-chat-oauth-refresh', + 'refresh-token', + expect.objectContaining({ httpOnly: true, maxAge: 30 * 24 * 60 * 60 * 1000 }), + ); // Redirects to the plain top-level URL — never to the inner-frame URL, which // would render editor-ui/the AS callback inside the sandboxed frame. expect(mockContext.getResponseObject().writeHead).toHaveBeenCalledWith(302, { @@ -361,29 +381,34 @@ describe('establishChatSessionIdentity', () => { const result = await establishChatSessionIdentity(mockContext, resourceUrl); - expect(result).toBe(false); + expect(result).toBeNull(); expect(mockContext.beginN8nOAuth2Flow).toHaveBeenCalledWith(resourceUrl); }); it('confirms readiness from the one-hop cookie without clearing it, leaving it for the frame', async () => { + const expiresAt = Date.now() + 3_600_000; mockContext.getRequestObject.mockReturnValue({ query: {}, - headers: { cookie: 'n8n-chat-oauth=as-token' }, + headers: { cookie: oauthCookie('as-token', expiresAt) }, originalUrl: '/webhook/abc/chat', } as never); mockContext.validateN8nOAuth2Token.mockResolvedValue({ valid: true, user }); const result = await establishChatSessionIdentity(mockContext, resourceUrl); - expect(result).toBe(true); + // Converted from the cookie's absolute expiry here, on the clock that wrote it, + // so no server timestamp reaches the shell. + expect(result?.expiresIn).toBeGreaterThan(3590); + expect(result?.expiresIn).toBeLessThanOrEqual(3600); expect(mockContext.validateN8nOAuth2Token).toHaveBeenCalledWith('as-token', resourceUrl); expect(mockContext.getResponseObject().clearCookie).not.toHaveBeenCalled(); + expect(mockContext.refreshN8nOAuth2Flow).not.toHaveBeenCalled(); }); it('restarts the flow when the one-hop cookie fails to validate', async () => { mockContext.getRequestObject.mockReturnValue({ query: {}, - headers: { cookie: 'n8n-chat-oauth=stale-token' }, + headers: { cookie: oauthCookie('stale-token', Date.now() + 3_600_000) }, originalUrl: '/webhook/abc/chat', } as never); mockContext.validateN8nOAuth2Token.mockResolvedValue({ valid: false, reason: 'invalid_token' }); @@ -391,10 +416,107 @@ describe('establishChatSessionIdentity', () => { const result = await establishChatSessionIdentity(mockContext, resourceUrl); - expect(result).toBe(false); + expect(result).toBeNull(); expect(mockContext.beginN8nOAuth2Flow).toHaveBeenCalledWith(resourceUrl); }); + // A reload mid-conversation: the 60-second one-hop cookie is long gone, but the + // grant behind the refresh cookie is still live, so rotating beats a full round + // trip back through the AS. + it('refreshes from the refresh cookie when the one-hop cookie has expired', async () => { + mockContext.getRequestObject.mockReturnValue({ + query: {}, + headers: { cookie: 'n8n-chat-oauth-refresh=refresh-token' }, + originalUrl: '/webhook/abc/chat', + } as never); + mockContext.refreshN8nOAuth2Flow.mockResolvedValue({ + valid: true, + token: 'fresh-token', + refreshToken: 'rotated-token', + expiresIn: 3600, + }); + + const result = await establishChatSessionIdentity(mockContext, resourceUrl); + + expect(mockContext.refreshN8nOAuth2Flow).toHaveBeenCalledWith('refresh-token', resourceUrl); + // The AS's own duration, handed on untouched — the shell schedules off its own clock. + expect(result).toEqual({ expiresIn: 3600 }); + expect(mockContext.beginN8nOAuth2Flow).not.toHaveBeenCalled(); + // Both cookies are rewritten: the frame's next GET needs the new access token, + // and the rotated refresh token replaces the one just consumed. + expect(mockContext.getResponseObject().cookie).toHaveBeenCalledWith( + 'n8n-chat-oauth', + expect.stringContaining('fresh-token'), + expect.objectContaining({ httpOnly: true }), + ); + expect(mockContext.getResponseObject().cookie).toHaveBeenCalledWith( + 'n8n-chat-oauth-refresh', + 'rotated-token', + expect.objectContaining({ httpOnly: true }), + ); + }); + + it('restarts the flow when the refresh cookie is refused', async () => { + mockContext.getRequestObject.mockReturnValue({ + query: {}, + headers: { cookie: 'n8n-chat-oauth-refresh=consumed-token' }, + originalUrl: '/webhook/abc/chat', + } as never); + mockContext.refreshN8nOAuth2Flow.mockResolvedValue({ + valid: false, + reason: 'invalid_grant', + }); + mockContext.beginN8nOAuth2Flow.mockResolvedValue('https://as.example.com/authorize'); + + const result = await establishChatSessionIdentity(mockContext, resourceUrl); + + expect(result).toBeNull(); + expect(mockContext.beginN8nOAuth2Flow).toHaveBeenCalledWith(resourceUrl); + }); + + // Nothing ever clears the refresh cookie, so a cookie the AS has finished with has + // to heal itself. It does: the refresh fails, the visitor is sent through the AS, + // and the callback writes a live token over the dead one. + it('overwrites a stale refresh cookie rather than needing it cleared', async () => { + mockContext.getRequestObject.mockReturnValue({ + query: {}, + headers: { cookie: 'n8n-chat-oauth-refresh=long-dead-token' }, + originalUrl: '/webhook/abc/chat', + } as never); + mockContext.refreshN8nOAuth2Flow.mockResolvedValue({ valid: false, reason: 'invalid_grant' }); + mockContext.beginN8nOAuth2Flow.mockResolvedValue('https://as.example.com/authorize'); + + expect(await establishChatSessionIdentity(mockContext, resourceUrl)).toBeNull(); + expect(mockContext.getResponseObject().clearCookie).not.toHaveBeenCalled(); + expect(mockContext.getResponseObject().writeHead).toHaveBeenCalledWith(302, { + Location: 'https://as.example.com/authorize', + }); + + // The AS auto-approves against the visitor's existing consent, so this leg is + // silent, and its callback replaces the dead cookie. + mockContext.getResponseObject.mockReturnValue(mockRes()); + mockContext.getRequestObject.mockReturnValue({ + query: { code: 'auth-code', state: 'flow-state' }, + headers: { cookie: 'n8n-chat-oauth-refresh=long-dead-token' }, + originalUrl: '/webhook/abc/chat?code=auth-code&state=flow-state', + } as never); + mockContext.completeN8nOAuth2Flow.mockResolvedValue({ + valid: true, + token: 'as-token', + refreshToken: 'brand-new-refresh-token', + expiresIn: 3600, + user, + }); + + expect(await establishChatSessionIdentity(mockContext, resourceUrl)).toBeNull(); + expect(mockContext.getResponseObject().cookie).toHaveBeenCalledWith( + 'n8n-chat-oauth-refresh', + 'brand-new-refresh-token', + expect.objectContaining({ httpOnly: true }), + ); + expect(mockContext.getResponseObject().clearCookie).not.toHaveBeenCalled(); + }); + it('reports denial without restarting the flow', async () => { mockContext.getRequestObject.mockReturnValue({ query: { error: 'access_denied' }, @@ -404,12 +526,142 @@ describe('establishChatSessionIdentity', () => { const result = await establishChatSessionIdentity(mockContext, resourceUrl); - expect(result).toBe(false); + expect(result).toBeNull(); expect(mockContext.getResponseObject().status).toHaveBeenCalledWith(403); expect(mockContext.beginN8nOAuth2Flow).not.toHaveBeenCalled(); }); }); +describe('handleChatTokenRefresh', () => { + const mockContext = mock(); + const resourceUrl = 'http://localhost:5678/webhook/abc/chat'; + + const mockRes = () => { + const res = { + status: vi.fn().mockReturnThis(), + json: vi.fn().mockReturnThis(), + end: vi.fn().mockReturnThis(), + setHeader: vi.fn().mockReturnThis(), + cookie: vi.fn().mockReturnThis(), + clearCookie: vi.fn().mockReturnThis(), + }; + return res as never; + }; + + beforeEach(() => { + vi.clearAllMocks(); + mockContext.getResponseObject.mockReturnValue(mockRes()); + mockContext.logger = { warn: vi.fn() } as never; + }); + + it('rotates the grant and returns only the access token and its lifetime', async () => { + mockContext.getRequestObject.mockReturnValue({ + headers: { cookie: 'n8n-chat-oauth-refresh=refresh-token' }, + } as never); + mockContext.refreshN8nOAuth2Flow.mockResolvedValue({ + valid: true, + token: 'fresh-token', + refreshToken: 'rotated-token', + expiresIn: 3600, + }); + + await handleChatTokenRefresh(mockContext, resourceUrl); + + const res = mockContext.getResponseObject(); + expect(mockContext.refreshN8nOAuth2Flow).toHaveBeenCalledWith('refresh-token', resourceUrl); + expect(res.status).toHaveBeenCalledWith(200); + expect(res.json).toHaveBeenCalledWith({ token: 'fresh-token', expiresIn: 3600 }); + // The rotated refresh token stays in its httpOnly cookie and never reaches the page. + expect(res.json).not.toHaveBeenCalledWith( + expect.objectContaining({ refreshToken: expect.anything() }), + ); + expect(res.cookie).toHaveBeenCalledWith( + 'n8n-chat-oauth-refresh', + 'rotated-token', + expect.objectContaining({ httpOnly: true }), + ); + expect(res.setHeader).toHaveBeenCalledWith('Cache-Control', 'no-store'); + }); + + it('answers 401 without calling the AS when there is no refresh cookie', async () => { + mockContext.getRequestObject.mockReturnValue({ headers: {} } as never); + + await handleChatTokenRefresh(mockContext, resourceUrl); + + expect(mockContext.refreshN8nOAuth2Flow).not.toHaveBeenCalled(); + expect(mockContext.getResponseObject().status).toHaveBeenCalledWith(401); + expect(mockContext.getResponseObject().json).toHaveBeenCalledWith({ error: 'invalid_grant' }); + }); + + // A token a concurrent refresh already consumed loses the AS's atomic rotation + // race. The page must be told to stop — but the cookie belongs to the winner by + // then, so clearing it would erase a live grant. + it('answers 401 without touching the refresh cookie when the AS refuses the token', async () => { + mockContext.getRequestObject.mockReturnValue({ + headers: { cookie: 'n8n-chat-oauth-refresh=consumed-token' }, + } as never); + mockContext.refreshN8nOAuth2Flow.mockResolvedValue({ + valid: false, + reason: 'invalid_grant', + }); + + await handleChatTokenRefresh(mockContext, resourceUrl); + + const res = mockContext.getResponseObject(); + expect(res.clearCookie).not.toHaveBeenCalled(); + expect(res.status).toHaveBeenCalledWith(401); + expect(res.json).toHaveBeenCalledWith({ error: 'invalid_grant' }); + }); + + // Two tabs on one chat page share the single cookie slot on that path. The loser + // must leave the winner's rotated token alone: it is the only copy of the grant. + it('leaves a cookie a concurrent shell just rotated intact', async () => { + mockContext.getRequestObject.mockReturnValue({ + headers: { cookie: 'n8n-chat-oauth-refresh=rotated-by-the-winner' }, + } as never); + mockContext.refreshN8nOAuth2Flow.mockResolvedValueOnce({ + valid: false, + reason: 'invalid_grant', + }); + + await handleChatTokenRefresh(mockContext, resourceUrl); + + const loserRes = mockContext.getResponseObject(); + expect(loserRes.status).toHaveBeenCalledWith(401); + expect(loserRes.clearCookie).not.toHaveBeenCalled(); + + // The loser's 5s retry now presents the same cookie, which the winner rotated. + mockContext.getResponseObject.mockReturnValue(mockRes()); + mockContext.refreshN8nOAuth2Flow.mockResolvedValueOnce({ + valid: true, + token: 'fresh-token', + refreshToken: 'rotated-again', + expiresIn: 3600, + }); + + await handleChatTokenRefresh(mockContext, resourceUrl); + + const retryRes = mockContext.getResponseObject(); + expect(retryRes.status).toHaveBeenCalledWith(200); + expect(retryRes.json).toHaveBeenCalledWith({ token: 'fresh-token', expiresIn: 3600 }); + expect(retryRes.clearCookie).not.toHaveBeenCalled(); + }); + + it('answers 401 rather than throwing when the AS call itself fails', async () => { + mockContext.getRequestObject.mockReturnValue({ + headers: { cookie: 'n8n-chat-oauth-refresh=refresh-token' }, + } as never); + mockContext.refreshN8nOAuth2Flow.mockRejectedValue(new Error('AS unreachable')); + + await expect(handleChatTokenRefresh(mockContext, resourceUrl)).resolves.toBeUndefined(); + + const res = mockContext.getResponseObject(); + expect(res.status).toHaveBeenCalledWith(401); + // A transient AS failure says nothing about the grant, so the cookie stays. + expect(res.clearCookie).not.toHaveBeenCalled(); + }); +}); + describe('resolveInnerFrameIdentity', () => { const mockContext = mock(); const resourceUrl = 'http://localhost:5678/webhook/abc/chat'; @@ -432,9 +684,13 @@ describe('resolveInnerFrameIdentity', () => { mockContext.getResponseObject.mockReturnValue(mockRes()); }); + /** The payload the one-hop cookie carries, percent-encoded as a browser sends it. */ + const oauthCookie = (token: string) => + `n8n-chat-oauth=${encodeURIComponent(JSON.stringify({ t: token, e: Date.now() + 3_600_000 }))}`; + it('resolves the visitor from the one-hop cookie and clears it', async () => { mockContext.getRequestObject.mockReturnValue({ - headers: { cookie: 'n8n-chat-oauth=as-token' }, + headers: { cookie: oauthCookie('as-token') }, } as never); mockContext.validateN8nOAuth2Token.mockResolvedValue({ valid: true, user }); @@ -448,6 +704,22 @@ describe('resolveInnerFrameIdentity', () => { ); }); + // Every later refresh the shell asks for is authenticated by the refresh cookie, + // so this render must leave it alone. + it('leaves the refresh cookie in place', async () => { + mockContext.getRequestObject.mockReturnValue({ + headers: { cookie: `${oauthCookie('as-token')}; n8n-chat-oauth-refresh=refresh-token` }, + } as never); + mockContext.validateN8nOAuth2Token.mockResolvedValue({ valid: true, user }); + + await resolveInnerFrameIdentity(mockContext, resourceUrl); + + expect(mockContext.getResponseObject().clearCookie).not.toHaveBeenCalledWith( + 'n8n-chat-oauth-refresh', + expect.anything(), + ); + }); + it('returns null, without starting a new flow, when there is no cookie', async () => { mockContext.getRequestObject.mockReturnValue({ headers: {} } as never); @@ -459,7 +731,7 @@ describe('resolveInnerFrameIdentity', () => { it('returns null, without starting a new flow, when the cookie fails to validate', async () => { mockContext.getRequestObject.mockReturnValue({ - headers: { cookie: 'n8n-chat-oauth=stale-token' }, + headers: { cookie: oauthCookie('stale-token') }, } as never); mockContext.validateN8nOAuth2Token.mockResolvedValue({ valid: false, reason: 'invalid_token' }); diff --git a/packages/@n8n/nodes-langchain/nodes/trigger/ChatTrigger/__test__/shell.test.ts b/packages/@n8n/nodes-langchain/nodes/trigger/ChatTrigger/__test__/shell.test.ts index e5ba1473083..6bb5d59cbb2 100644 --- a/packages/@n8n/nodes-langchain/nodes/trigger/ChatTrigger/__test__/shell.test.ts +++ b/packages/@n8n/nodes-langchain/nodes/trigger/ChatTrigger/__test__/shell.test.ts @@ -1,12 +1,16 @@ import type { Request, Response } from 'express'; import { + buildChatRefreshUrl, buildInnerFrameSrc, clearChatOAuthToken, isChatOAuth2Enabled, + isChatRefreshRequest, isShellInnerRequest, readChatOAuthToken, + readChatRefreshToken, setChatOAuthToken, + setChatRefreshToken, } from '../shell'; const request = (overrides: Partial = {}) => @@ -88,8 +92,22 @@ describe('buildInnerFrameSrc', () => { }); }); +describe('buildChatRefreshUrl', () => { + it('flags the same endpoint as the refresh leg', () => { + expect(buildChatRefreshUrl(request())).toBe('/webhook/abc/chat?n8nChatRefresh=1'); + }); + + it('keeps the page own query parameters', () => { + const req = request({ originalUrl: '/webhook-test/abc/chat?foo=bar' }); + + expect(buildChatRefreshUrl(req)).toBe('/webhook-test/abc/chat?foo=bar&n8nChatRefresh=1'); + }); +}); + describe('chat OAuth2 one-hop cookie', () => { const resourceUrl = 'http://localhost:5678/webhook/abc/chat'; + const payload = { token: 'as-token', expiresAt: 1_700_000_000_000 }; + const serialized = JSON.stringify({ t: 'as-token', e: 1_700_000_000_000 }); const response = () => ({ @@ -101,9 +119,9 @@ describe('chat OAuth2 one-hop cookie', () => { const req = request({ headers: { host: 'localhost:5678' }, protocol: 'http' }); const res = response(); - setChatOAuthToken(res, req, resourceUrl, 'as-token'); + setChatOAuthToken(res, req, resourceUrl, payload); - expect(res.cookie).toHaveBeenCalledWith('n8n-chat-oauth', 'as-token', { + expect(res.cookie).toHaveBeenCalledWith('n8n-chat-oauth', serialized, { httpOnly: true, sameSite: 'lax', secure: false, @@ -116,11 +134,11 @@ describe('chat OAuth2 one-hop cookie', () => { const req = request({ headers: { 'x-forwarded-proto': 'https' }, protocol: 'http' }); const res = response(); - setChatOAuthToken(res, req, resourceUrl, 'as-token'); + setChatOAuthToken(res, req, resourceUrl, payload); expect(res.cookie).toHaveBeenCalledWith( 'n8n-chat-oauth', - 'as-token', + serialized, expect.objectContaining({ secure: true }), ); }); @@ -135,11 +153,11 @@ describe('chat OAuth2 one-hop cookie', () => { }); const res = response(); - setChatOAuthToken(res, req, resourceUrl, 'as-token'); + setChatOAuthToken(res, req, resourceUrl, payload); expect(res.cookie).toHaveBeenCalledWith( 'n8n-chat-oauth', - 'as-token', + serialized, expect.objectContaining({ secure: true }), ); }); @@ -151,19 +169,23 @@ describe('chat OAuth2 one-hop cookie', () => { }); const res = response(); - setChatOAuthToken(res, req, resourceUrl, 'as-token'); + setChatOAuthToken(res, req, resourceUrl, payload); expect(res.cookie).toHaveBeenCalledWith( 'n8n-chat-oauth', - 'as-token', + serialized, expect.objectContaining({ secure: true }), ); }); - it('reads the cookie back from the raw header', () => { - const req = request({ headers: { cookie: 'other=1; n8n-chat-oauth=as-token; more=2' } }); + // The shell schedules its refresh off the expiry, so it has to survive the round + // trip through the cookie exactly as written. + it('round-trips the token and its expiry', () => { + const req = request({ + headers: { cookie: `other=1; n8n-chat-oauth=${encodeURIComponent(serialized)}; more=2` }, + }); - expect(readChatOAuthToken(req)).toBe('as-token'); + expect(readChatOAuthToken(req)).toEqual(payload); }); it('returns null when the cookie is absent', () => { @@ -171,10 +193,12 @@ describe('chat OAuth2 one-hop cookie', () => { expect(readChatOAuthToken(request())).toBeNull(); }); - it('decodes a percent-encoded value', () => { - const req = request({ headers: { cookie: 'n8n-chat-oauth=a%2Fb' } }); + // The refresh cookie's name begins with this one's, so a request carrying only the + // refresh cookie must not be read as a payload. + it('does not read the refresh cookie as the one-hop payload', () => { + const req = request({ headers: { cookie: 'n8n-chat-oauth-refresh=refresh-token' } }); - expect(readChatOAuthToken(req)).toBe('a/b'); + expect(readChatOAuthToken(req)).toBeNull(); }); it('treats an undecodable value as no cookie', () => { @@ -183,6 +207,20 @@ describe('chat OAuth2 one-hop cookie', () => { expect(readChatOAuthToken(req)).toBeNull(); }); + // Anything that isn't the payload shape — a bare token from an older build, a + // truncated value — must not schedule a refresh off a number we invented. + it.each([ + ['a bare token', 'as-token'], + ['a payload with no expiry', '{"t":"as-token"}'], + ['a payload with no token', '{"e":1700000000000}'], + ['a payload with an empty token', '{"t":"","e":1700000000000}'], + ['a non-finite expiry', '{"t":"as-token","e":null}'], + ])('treats %s as no cookie', (_label, value) => { + const req = request({ headers: { cookie: `n8n-chat-oauth=${encodeURIComponent(value)}` } }); + + expect(readChatOAuthToken(req)).toBeNull(); + }); + it('clears the cookie scoped to the same path', () => { const req = request({ headers: { host: 'localhost:5678' }, protocol: 'http' }); const res = response(); @@ -197,3 +235,97 @@ describe('chat OAuth2 one-hop cookie', () => { }); }); }); + +describe('chat OAuth2 refresh cookie', () => { + const resourceUrl = 'http://localhost:5678/webhook/abc/chat'; + + const response = () => + ({ + cookie: vi.fn(), + clearCookie: vi.fn(), + }) as unknown as Response; + + // httpOnly is the whole design: no document and no script may read this value. + // 30 days matches the AS's own refresh-token life, so the cookie never outlives + // the grant it names. + it('sets an httpOnly, path-scoped, 30-day cookie', () => { + const req = request({ headers: { host: 'localhost:5678' }, protocol: 'http' }); + const res = response(); + + setChatRefreshToken(res, req, resourceUrl, 'refresh-token'); + + expect(res.cookie).toHaveBeenCalledWith('n8n-chat-oauth-refresh', 'refresh-token', { + httpOnly: true, + sameSite: 'lax', + secure: false, + path: '/webhook/abc/chat', + maxAge: 30 * 24 * 60 * 60 * 1000, + }); + }); + + it('marks the cookie secure over https (honouring x-forwarded-proto)', () => { + const req = request({ headers: { 'x-forwarded-proto': 'https' }, protocol: 'http' }); + const res = response(); + + setChatRefreshToken(res, req, resourceUrl, 'refresh-token'); + + expect(res.cookie).toHaveBeenCalledWith( + 'n8n-chat-oauth-refresh', + 'refresh-token', + expect.objectContaining({ secure: true }), + ); + }); + + it('reads the cookie back alongside the one-hop cookie', () => { + const req = request({ + headers: { cookie: 'n8n-chat-oauth=%7B%7D; n8n-chat-oauth-refresh=refresh-token' }, + }); + + expect(readChatRefreshToken(req)).toBe('refresh-token'); + }); + + it('returns null when the cookie is absent', () => { + expect(readChatRefreshToken(request({ headers: { cookie: 'n8n-chat-oauth=x' } }))).toBeNull(); + }); +}); + +describe('isChatRefreshRequest', () => { + const refreshRequest = (overrides: Partial = {}) => + request({ + query: { n8nChatRefresh: '1' }, + headers: { 'x-n8n-chat-refresh': '1', 'sec-fetch-site': 'same-origin' }, + ...overrides, + }); + + it('accepts the shell own same-origin fetch', () => { + expect(isChatRefreshRequest(refreshRequest())).toBe(true); + }); + + it('is false without the query flag', () => { + expect(isChatRefreshRequest(refreshRequest({ query: {} }))).toBe(false); + }); + + // The custom header is the CSRF guard: another origin cannot set it without a + // preflight this endpoint never answers. A plain forged GET has to be refused. + it('refuses a request with no custom header', () => { + expect( + isChatRefreshRequest(refreshRequest({ headers: { 'sec-fetch-site': 'same-origin' } })), + ).toBe(false); + }); + + it.each(['cross-site', 'same-site', 'none'])('refuses Sec-Fetch-Site %s', (site) => { + expect( + isChatRefreshRequest( + refreshRequest({ headers: { 'x-n8n-chat-refresh': '1', 'sec-fetch-site': site } }), + ), + ).toBe(false); + }); + + // A proxy that strips Sec-Fetch-Site must not break the leg; the custom header + // still stands. + it('accepts a request with no Sec-Fetch-Site at all', () => { + expect(isChatRefreshRequest(refreshRequest({ headers: { 'x-n8n-chat-refresh': '1' } }))).toBe( + true, + ); + }); +}); diff --git a/packages/@n8n/nodes-langchain/nodes/trigger/ChatTrigger/__test__/templates.test.ts b/packages/@n8n/nodes-langchain/nodes/trigger/ChatTrigger/__test__/templates.test.ts index 4f1567869c8..e0ebe6173f5 100644 --- a/packages/@n8n/nodes-langchain/nodes/trigger/ChatTrigger/__test__/templates.test.ts +++ b/packages/@n8n/nodes-langchain/nodes/trigger/ChatTrigger/__test__/templates.test.ts @@ -657,6 +657,158 @@ describe('createShellPage', () => { expect(shell).toContain("'n8n-chat-shell/sessionId' + window.location.pathname"); expect(shell).toContain("'#sessionId=' + encodeURIComponent(sessionId)"); }); + + // With the OAuth path off there is no token to keep alive, so the shell must be + // exactly the document it was before refresh existed. + it('carries no refresh machinery when no refresh is passed', () => { + expect(shell).not.toContain('n8nChatRefresh'); + expect(shell).not.toContain('n8n-chat-auth-token'); + expect(shell).not.toContain('n8n-chat-frame-ready'); + expect(shell).not.toContain('MessageChannel'); + expect(shell).not.toContain('fetch('); + }); + + // The refresh script had to move ahead of this one so its listener is installed + // while the frame is still about:blank. With refresh absent the document must be + // byte-for-byte the one it was before refresh existed. + it('places the frame script exactly where it was before refresh existed', () => { + expect(shell).toContain('>\n\t\t\n\t'); + }); +}); + +describe('createShellPage with token refresh', () => { + const shell = createShellPage({ + iframeSrc: '/webhook/abc/chat?n8nShellInner=1', + refresh: { url: '/webhook/abc/chat?n8nChatRefresh=1', expiresIn: 3600 }, + }); + + it('schedules ahead of the lifetime it was given', () => { + expect(shell).toContain('planFor(3600)'); + expect(shell).toContain('setTimeout'); + // The lead is the margin BEFORE expiry, not the refresh time: a fifth of the + // lifetime clamped to [60s, 600s], so a one-hour token refreshes at t+50min. + expect(shell).toContain('Math.min(600, Math.max(60, lifetimeSeconds * 0.2))'); + }); + + // The timer and its one retry are the only things that start a refresh, so two can + // never be in flight and the script needs no concurrency guard. A second trigger — + // a visibility or focus listener — would race the timer over the single refresh + // cookie on this path, so it cannot be added without a latch. + it('starts a refresh from the timer alone', () => { + expect(shell).toContain('timer = setTimeout(function () { refresh(false); }, delay);'); + expect(shell).not.toContain('visibilitychange'); + expect(shell).not.toContain('inFlight'); + }); + + // An absolute expiry the server computed, compared against the page's own + // `Date.now()`, is wrong by however far the two clocks disagree — and the page has + // no way to detect that. Every reading the schedule makes must come from one clock, + // so the server converts to a duration before it reaches the document. + it('never interpolates a server timestamp into the document', () => { + const rendered = createShellPage({ + iframeSrc: '/webhook/abc/chat?n8nShellInner=1', + refresh: { url: '/webhook/abc/chat?n8nChatRefresh=1', expiresIn: 3600 }, + }); + + // A 13-digit epoch-ms literal is what a leaked `expiresAt` would look like. + expect(rendered).not.toMatch(/\b1[0-9]{12}\b/); + }); + + // Anchoring the new lifetime to when the response *arrived* always overstates what + // is left, by however long the round trip took — a slow leg, or a paused server — + // and overstating is the direction that ends in 401s. + it('subtracts the round trip it measured before rescheduling', () => { + expect(shell).toContain('var startedAt = Date.now();'); + expect(shell).toContain('planFor(lifetime - (Date.now() - startedAt) / 1000);'); + }); + + it('rounds a fractional lifetime and never emits a negative one', () => { + const soon = createShellPage({ + iframeSrc: '/x', + refresh: { url: '/x?n8nChatRefresh=1', expiresIn: 0 }, + }); + + expect(soon).toContain('planFor(0)'); + expect(soon).not.toContain('planFor(-'); + }); + + it('fetches the refresh leg with the custom header that guards it', () => { + expect(shell).toContain('"/webhook/abc/chat?n8nChatRefresh=1"'); + expect(shell).toContain("'x-n8n-chat-refresh': '1'"); + expect(shell).toContain("credentials: 'same-origin'"); + }); + + // `frame.contentWindow` names the browsing context, not the document, so it keeps + // resolving after author script navigates the frame away — and would hand the next + // rotated token to whatever loaded there. A port dies with the document that made it. + it('hands the token down a channel the frame opened, never at its window', () => { + expect(shell).toContain("port.postMessage({ type: 'n8n-chat-auth-token', token: token })"); + expect(shell).toContain('deliver(data.token);'); + expect(shell).not.toContain('contentWindow.postMessage'); + }); + + // Two parser-inserted scripts do not run in one uninterrupted turn, so a listener + // installed after the frame is navigated could miss the announcement. Installed + // first, it is in place while the frame is still about:blank — which, sandboxed + // without allow-same-origin, has no script and cannot post. + it('installs the ready listener before it navigates the frame', () => { + expect(shell.indexOf("addEventListener('message'")).toBeGreaterThan(-1); + expect(shell.indexOf("addEventListener('message'")).toBeLessThan( + shell.indexOf("frame.src = frame.getAttribute('data-src')"), + ); + }); + + it('accepts a ready message only from the frame itself', () => { + expect(shell).toContain('if (!frame || event.source !== frame.contentWindow) return;'); + expect(shell).toContain("data.type !== 'n8n-chat-frame-ready'"); + }); + + it('latches the first ready message and never a later one', () => { + expect(shell).toContain('if (latched) return;'); + expect(shell).toContain('var latched = false;'); + expect(shell).toContain('latched = true;'); + // Nothing ever sets it back: the only assignment to false is the declaration. + expect(shell.match(/latched = false/g)).toHaveLength(1); + }); + + // A browser with no MessageChannel announces itself without a port. That still has + // to close the latch, or a document that later replaces the frame could claim it. + it('closes the latch on a ready message that carries no port', () => { + expect(shell.indexOf('latched = true;')).toBeLessThan( + shell.indexOf('if (event.ports && event.ports.length) port = event.ports[0];'), + ); + }); + + // With no port there is nowhere to put a fresh token, and the frame's own baked-in + // token lasts its full lifetime — exactly the pre-refresh behaviour. + it('stops refreshing when the frame announces readiness without a port', () => { + expect(shell).toContain('if (latched && !port) return;'); + expect(shell).toContain('if (timer) { clearTimeout(timer); timer = null; }'); + }); + + // A refresh can beat the frame's bootstrap, and the post is one-shot. + it('holds the newest token until the port arrives', () => { + expect(shell).toContain('pendingToken = token;'); + expect(shell).toContain( + "port.postMessage({ type: 'n8n-chat-auth-token', token: pendingToken })", + ); + expect(shell).toContain("pendingToken = '';"); + // If no port ever arrives, reload rather than fall back to the frame's window. + expect(shell).toContain('portTimer = setTimeout(portMissing, 10000);'); + }); + + it('retries once and then reloads exactly once', () => { + expect(shell).toContain('refresh(true)'); + expect(shell).toContain('window.location.reload()'); + expect(shell).toContain('if (reloaded) return;'); + }); + + // The whole point of the httpOnly cookie: the refresh token exists in no document. + it('carries no refresh token', () => { + expect(shell).not.toContain('refreshToken'); + expect(shell).not.toContain('n8n-chat-oauth-refresh'); + }); }); describe('createPage inside the shell frame', () => { @@ -702,7 +854,47 @@ describe('createPage inside the shell frame', () => { }); it('authenticates messages by request header', () => { - expect(inner).toContain('\'x-auth-token\': "signed.jwt.token",'); + expect(inner).toContain('headers[\'x-auth-token\'] = "signed.jwt.token"'); + }); + + // `createChat` keeps this object by reference and the widget reads it on every + // send, so a later in-place write is what makes a refreshed token take effect. + it('hands the widget a header object it can keep mutating', () => { + expect(inner).toContain('const headers = window.__n8nChatAuthHeaders || {};'); + expect(inner).toContain('headers: headers'); + expect(inner).toContain("headers['X-Instance-Id'] = 'test-instance';"); + // Guards the race where a refresh lands before this module script runs. + expect(inner).toContain("if (!headers['x-auth-token'])"); + }); + + // Not a window listener: a port belongs to this document's realm, so it dies with + // this document. A replacement loaded by author script cannot obtain it, which is + // what keeps the shell's next token away from it. + it('opens a private channel to the shell instead of listening on window', () => { + expect(inner).toContain('window.__n8nChatAuthHeaders = {};'); + expect(inner).toContain('var channel = new MessageChannel();'); + expect(inner).toContain('channel.port1.onmessage = function (event) {'); + expect(inner).toContain( + "window.parent.postMessage({ type: 'n8n-chat-frame-ready' }, '*', [channel.port2]);", + ); + expect(inner).toContain("data.type !== 'n8n-chat-auth-token'"); + expect(inner).toContain("window.__n8nChatAuthHeaders['x-auth-token'] = data.token;"); + expect(inner).not.toContain("addEventListener('message'"); + }); + + // Announcing with no port still closes the shell's latch, so a document loaded here + // later cannot claim the channel this one failed to open. + it('announces readiness without a port when the browser has no channel', () => { + expect(inner).toContain( + "try { window.parent.postMessage({ type: 'n8n-chat-frame-ready' }, '*'); } catch (postError) {}", + ); + }); + + // The refresh token lives only in an httpOnly cookie; it must appear in neither + // document. + it('carries no refresh token', () => { + expect(inner).not.toContain('refreshToken'); + expect(inner).not.toContain('n8n-chat-oauth-refresh'); }); // Not merely skipped at runtime: the bootstrap is never emitted, so there is no @@ -728,6 +920,14 @@ describe('createPage inside the shell frame', () => { expect(plain).toContain('const injectedVisitor = null;'); }); + // Nothing refreshes this render, so it keeps the inline header literal rather + // than the mutable object the frame render needs. + it('keeps the header literal inline', () => { + expect(plain).toContain("'X-Instance-Id': 'test-instance',"); + expect(plain).not.toContain('window.__n8nChatAuthHeaders'); + expect(plain).not.toContain('headers: headers'); + }); + // The client-side bootstrap is what the flag-off n8nUserAuth render still relies on. it('keeps the login bootstrap the flag-off render depends on', () => { expect(plain).toContain("fetch('/rest/login'"); diff --git a/packages/@n8n/nodes-langchain/nodes/trigger/ChatTrigger/shell.ts b/packages/@n8n/nodes-langchain/nodes/trigger/ChatTrigger/shell.ts index 0380ceed619..a69d98a4355 100644 --- a/packages/@n8n/nodes-langchain/nodes/trigger/ChatTrigger/shell.ts +++ b/packages/@n8n/nodes-langchain/nodes/trigger/ChatTrigger/shell.ts @@ -36,13 +36,45 @@ export function buildInnerFrameSrc(req: Request): string { return `${path}?${params.toString()}`; } +/** Query flag that asks the `setup` GET for the token-refresh leg instead of a page. */ +export const CHAT_REFRESH_PARAM = 'n8nChatRefresh'; + +/** + * Custom header the shell's refresh `fetch` sets. A cross-origin page cannot set it + * without a CORS preflight this endpoint never answers, so requiring it is what stops + * another site from driving the leg with the visitor's cookies. + */ +export const CHAT_REFRESH_HEADER = 'x-n8n-chat-refresh'; + +/** Relative, so the shell's refresh `fetch` stays same-origin behind any host or prefix. */ +export function buildChatRefreshUrl(req: Request): string { + const [path, query] = req.originalUrl.split('?'); + const params = new URLSearchParams(query); + params.set(CHAT_REFRESH_PARAM, '1'); + return `${path}?${params.toString()}`; +} + // Carries the AS access token across the single same-site redirect from the AS // callback to the clean inner-frame URL, so `code`/`state` never reach the // author-shaped chat widget. The token is otherwise already embedded in the // frame's HTML (sent back as `x-auth-token` on every message), so this cookie -// is not a new exposure. +// is not a new exposure. The expiry rides along because the shell has to know +// when to refresh and the frame's HTML carries the token alone. const CHAT_OAUTH_COOKIE_NAME = 'n8n-chat-oauth'; +/** + * Holds the grant's refresh token for the life of the hosted page. `httpOnly` is the + * whole point: neither document may read it, so it never enters any HTML and never + * enters any script scope. Only the refresh leg, server-side, ever sees it. + * + * Names must stay in step with `CHAT_OAUTH_REFRESH_COOKIE_NAME` in + * `packages/cli/src/constants.ts`, which strips both from every other webhook. + */ +const CHAT_OAUTH_REFRESH_COOKIE_NAME = 'n8n-chat-oauth-refresh'; + +/** Matches `REFRESH_TOKEN_EXPIRY_MS` in the AS: the cookie must not outlive the grant. */ +const REFRESH_COOKIE_MAX_AGE_MS = 30 * 24 * 60 * 60 * 1000; + /** * Derive `secure` from the request scheme (honouring x-forwarded-proto) rather * than config, so the cookie is actually sent back over http in dev while @@ -67,13 +99,21 @@ function chatOAuthCookieOptions(req: Request, resourceUrl: string) { }; } +/** What the one-hop cookie carries: the access token and its absolute expiry in ms. */ +export type ChatOAuthCookiePayload = { token: string; expiresAt: number }; + +// Short keys because a cookie value is capped at ~4KB and the token already +// takes most of it. +type SerializedPayload = { t: string; e: number }; + export function setChatOAuthToken( res: Response, req: Request, resourceUrl: string, - token: string, + payload: ChatOAuthCookiePayload, ): void { - res.cookie(CHAT_OAUTH_COOKIE_NAME, token, { + const value: SerializedPayload = { t: payload.token, e: payload.expiresAt }; + res.cookie(CHAT_OAUTH_COOKIE_NAME, JSON.stringify(value), { ...chatOAuthCookieOptions(req, resourceUrl), maxAge: 60_000, // one redirect hop; short by design }); @@ -92,11 +132,73 @@ function decodeCookieValue(value: string): string | null { } } -export function readChatOAuthToken(req: Request): string | null { - const match = (req.headers.cookie ?? '').match(/(?:^|;\s*)n8n-chat-oauth=([^;]+)/); - return match ? decodeCookieValue(match[1]) : null; +/** + * Read one cookie by exact name. Split rather than matched, so `n8n-chat-oauth` can't + * pick up `n8n-chat-oauth-refresh`, whose name begins with it. + */ +function readRawCookie(req: Request, name: string): string | null { + for (const pair of (req.headers.cookie ?? '').split(';')) { + const separator = pair.indexOf('='); + if (separator === -1) continue; + if (pair.slice(0, separator).trim() !== name) continue; + return decodeCookieValue(pair.slice(separator + 1)); + } + return null; +} + +function isSerializedPayload(value: unknown): value is SerializedPayload { + if (typeof value !== 'object' || value === null) return false; + const { t, e } = value as Partial; + return typeof t === 'string' && t !== '' && typeof e === 'number' && Number.isFinite(e); +} + +export function readChatOAuthToken(req: Request): ChatOAuthCookiePayload | null { + const raw = readRawCookie(req, CHAT_OAUTH_COOKIE_NAME); + if (!raw) return null; + // Anything that isn't the payload shape — a cookie from another writer, a truncated + // value — is treated as absent, so the caller restarts the flow instead of + // scheduling off a number it invented. + try { + const parsed: unknown = JSON.parse(raw); + return isSerializedPayload(parsed) ? { token: parsed.t, expiresAt: parsed.e } : null; + } catch { + return null; + } } export function clearChatOAuthToken(res: Response, req: Request, resourceUrl: string): void { res.clearCookie(CHAT_OAUTH_COOKIE_NAME, chatOAuthCookieOptions(req, resourceUrl)); } + +export function setChatRefreshToken( + res: Response, + req: Request, + resourceUrl: string, + refreshToken: string, +): void { + res.cookie(CHAT_OAUTH_REFRESH_COOKIE_NAME, refreshToken, { + ...chatOAuthCookieOptions(req, resourceUrl), + maxAge: REFRESH_COOKIE_MAX_AGE_MS, + }); +} + +export function readChatRefreshToken(req: Request): string | null { + return readRawCookie(req, CHAT_OAUTH_REFRESH_COOKIE_NAME); +} + +/** + * Whether this GET is the shell asking for a fresh access token rather than for a page. + * + * A GET is required because a POST to this path reaches the `default` webhook — the + * chat message endpoint — not `setup`. That makes the request forgeable by shape alone, + * so the custom header carries the CSRF guard: it needs a preflight no other origin can + * get past. `Sec-Fetch-Site` is a second check where the browser sends it, and is + * ignored when absent so a stripping proxy doesn't break the leg. + */ +export function isChatRefreshRequest(req: Request): boolean { + if (req.query[CHAT_REFRESH_PARAM] !== '1') return false; + if (req.headers[CHAT_REFRESH_HEADER] !== '1') return false; + const site = req.headers['sec-fetch-site']; + if (typeof site === 'string' && site !== 'same-origin') return false; + return true; +} diff --git a/packages/@n8n/nodes-langchain/nodes/trigger/ChatTrigger/templates.ts b/packages/@n8n/nodes-langchain/nodes/trigger/ChatTrigger/templates.ts index 5da20b3d537..bb7d1456feb 100644 --- a/packages/@n8n/nodes-langchain/nodes/trigger/ChatTrigger/templates.ts +++ b/packages/@n8n/nodes-langchain/nodes/trigger/ChatTrigger/templates.ts @@ -85,10 +85,12 @@ export function getSanitizedCustomCss(customCss: string): string { const WIDGET_SESSION_ID_KEY = 'n8n-chat/sessionId'; /** - * Runs before the widget's module script (classic inline scripts aren't deferred). Both - * jobs follow from the frame having no origin: stand in for `localStorage`, which the - * widget touches at startup and which throws here, and read the session id the shell - * passes in the fragment. + * Runs before the widget's module script (classic inline scripts aren't deferred). The + * first two jobs follow from the frame having no origin: stand in for `localStorage`, + * which the widget touches at startup and which throws here, and read the session id the + * shell passes in the fragment. The third is the auth channel — this document opens the + * `MessagePort` the shell delivers rotated tokens down, so the channel belongs to this + * document and no later one can inherit it. */ const innerBootstrapScript = ` `; @@ -122,7 +150,19 @@ const innerBootstrapScript = ` * the frame. Everything the author can shape lives in that frame, which has no origin * and so can't reach this document's cookies or the OAuth `BroadcastChannel`. */ -export function createShellPage({ iframeSrc }: { iframeSrc: string }) { +export function createShellPage({ + iframeSrc, + refresh, +}: { + iframeSrc: string; + /** + * Set only on the OAuth2 path: where to ask for a fresh access token, and how many + * seconds the one the frame was just handed has left. A duration, never an absolute + * timestamp — see `ChatShellSession`. Absent leaves the shell exactly what it was + * before refresh existed. + */ + refresh?: { url: string; expiresIn: number }; +}) { return ` @@ -140,7 +180,7 @@ export function createShellPage({ iframeSrc }: { iframeSrc: string }) { title="Chat" sandbox="${CHAT_FRAME_SANDBOX}" data-src="${escapeForHtmlAttribute(iframeSrc)}" - > + >${refresh ? refreshScript(refresh) : ''} `; +} + export function createPage({ instanceId, webhookUrl, @@ -273,6 +452,26 @@ export function createPage({ email: frameIdentity.visitor.email, })} };`; + // In the frame, the header object is hoisted out of the `createChat` literal so a + // reference to it survives the call: `createChat` keeps this object's identity and + // the widget reads it on every send, so the shell's refresh writes the rotated token + // into it in place and nothing re-enters this code. The `if` covers the narrow race + // where a refresh lands before this module script runs. The unsplit render keeps the + // literal inline so its page stays byte-for-byte what it was. + const headersBootstrap = frameIdentity + ? `const headers = window.__n8nChatAuthHeaders || {}; + headers['X-Instance-Id'] = '${instanceId}'; + if (!headers['x-auth-token']) headers['x-auth-token'] = ${escapeForScriptContext(frameIdentity.authToken)}; + + ` + : ''; + const webhookConfigHeaders = frameIdentity + ? 'headers: headers' + : `headers: { + 'X-Instance-Id': '${instanceId}', + + }`; + return ` @@ -298,7 +497,7 @@ export function createPage({ (async function () { ${identityBootstrap} - createChat({ + ${headersBootstrap}createChat({ mode: 'fullscreen', webhookUrl: ${escapeForScriptContext(webhookUrl ?? '')}, showWelcomeScreen: ${sanitizedShowWelcomeScreen}, @@ -306,10 +505,7 @@ export function createPage({ metadata: metadata, ${shellInner ? 'sessionId: window.__n8nChatSessionId || undefined,' : ''} webhookConfig: { - headers: { - 'X-Instance-Id': '${instanceId}', - ${frameIdentity ? `'x-auth-token': ${escapeForScriptContext(frameIdentity.authToken)},` : ''} - } + ${webhookConfigHeaders} }, allowFileUploads: ${sanitizedAllowFileUploads}, allowedFilesMimeTypes: ${escapeForScriptContext(sanitizedAllowedFilesMimeTypes)}, diff --git a/packages/@n8n/nodes-langchain/nodes/trigger/ChatTrigger/types.ts b/packages/@n8n/nodes-langchain/nodes/trigger/ChatTrigger/types.ts index 4a5bbf0c4a8..d1442bd7a92 100644 --- a/packages/@n8n/nodes-langchain/nodes/trigger/ChatTrigger/types.ts +++ b/packages/@n8n/nodes-langchain/nodes/trigger/ChatTrigger/types.ts @@ -13,6 +13,22 @@ export type LoadPreviousSessionChatOption = (typeof validOptions)[number]; */ export type ChatFrameIdentity = { visitor: IUser; authToken: string }; +/** + * What the trusted shell needs to keep the frame's token alive: how much longer the + * access token it just handed the frame will work, in seconds. + * + * A duration, not an absolute `expiresAt`, because this value is about to be written + * into a document and read by the browser's clock. An absolute server timestamp + * compared against `Date.now()` in the page is wrong by however far the two machines' + * clocks disagree; a duration is wrong only by the time in flight, which the page can + * measure and subtract. `expiresAt` stays inside `shell.ts`, where both ends of the + * cookie hop read the same clock. + * + * The refresh token is deliberately absent — it lives only in an httpOnly cookie, so + * no document and no script ever holds it. + */ +export type ChatShellSession = { expiresIn: number }; + function isValidLoadPreviousSessionOption(value: unknown): value is LoadPreviousSessionChatOption { return typeof value === 'string' && (validOptions as readonly string[]).includes(value); } diff --git a/packages/cli/src/constants.ts b/packages/cli/src/constants.ts index 156a132fa89..bc2a9592b94 100644 --- a/packages/cli/src/constants.ts +++ b/packages/cli/src/constants.ts @@ -87,6 +87,16 @@ export const OIDC_NONCE_COOKIE_NAME = 'n8n-oidc-nonce'; export const FORM_AUTH_COOKIE_PREFIX = 'n8n-form-auth'; export const FORM_OAUTH_COOKIE_NAME = 'n8n-form-oauth'; +/** + * Cookies the Chat trigger's hosted page sets, duplicated here for the same reason + * as the form ones above — the names are owned by + * `@n8n/n8n-nodes-langchain/nodes/trigger/ChatTrigger/shell.ts`. Keep both sides in + * step. The `-refresh` one carries a 30-day refresh token, so it must never leak to + * an unrelated webhook. + */ +export const CHAT_OAUTH_COOKIE_NAME = 'n8n-chat-oauth'; +export const CHAT_OAUTH_REFRESH_COOKIE_NAME = 'n8n-chat-oauth-refresh'; + export const NPM_COMMAND_TOKENS = { NPM_PACKAGE_NOT_FOUND_ERROR: '404 Not Found', NPM_PACKAGE_VERSION_NOT_FOUND_ERROR: 'No matching version found for', diff --git a/packages/cli/src/modules/oauth-server/__tests__/oauth-flow.service.api.test.ts b/packages/cli/src/modules/oauth-server/__tests__/oauth-flow.service.api.test.ts index bdfc920dcb0..372c08f7629 100644 --- a/packages/cli/src/modules/oauth-server/__tests__/oauth-flow.service.api.test.ts +++ b/packages/cli/src/modules/oauth-server/__tests__/oauth-flow.service.api.test.ts @@ -17,6 +17,7 @@ import { UrlService } from '@/services/url.service'; import { createOwner, createMember } from '@test-integration/db/users'; import { setupTestServer } from '@test-integration/utils'; +import { RefreshTokenRepository } from '../database/repositories/oauth-refresh-token.repository'; import { OAuthAuthorizationCodeService } from '../oauth-authorization-code.service'; import { OAuth2FlowService } from '../oauth-flow.service'; import { OAuthServerService } from '../oauth-server.service'; @@ -188,6 +189,22 @@ describe('complete', () => { } }); + // The AS already mints and persists these; a caller that never sees them can only + // restart the whole flow when the access token expires. + test('returns the refresh token and the access token lifetime', async () => { + const resourceUrl = await createProtectedFormWorkflow(); + const { code, state } = await authorizeAndMintCode(resourceUrl, owner.id); + + const result = await flow.complete(code, state); + + expect(result.valid).toBe(true); + if (result.valid) { + expect(result.refreshToken).toEqual(expect.any(String)); + expect(result.refreshToken).not.toBe(result.token); + expect(result.expiresIn).toBeGreaterThan(0); + } + }); + test('returns metadata stashed at begin, and undefined when none was stashed', async () => { const resourceUrl = await createProtectedFormWorkflow(); @@ -275,6 +292,115 @@ describe('complete', () => { }); }); +describe('refresh', () => { + /** Complete a flow and hand back the refresh token it issued. */ + const grantFor = async (resourceUrl: string, userId = owner.id) => { + const { code, state } = await authorizeAndMintCode(resourceUrl, userId); + const result = await flow.complete(code, state); + if (!result.valid) throw new Error(`expected a valid flow, got ${result.reason}`); + return result; + }; + + test('rotates into a fresh pair bound to the same grant', async () => { + const resourceUrl = await createProtectedFormWorkflow(); + const granted = await grantFor(resourceUrl); + + const refreshed = await flow.refreshVirtualClientToken(granted.refreshToken, resourceUrl); + + expect(refreshed.valid).toBe(true); + if (refreshed.valid) { + expect(refreshed.token).not.toBe(granted.token); + expect(refreshed.refreshToken).not.toBe(granted.refreshToken); + expect(refreshed.expiresIn).toBeGreaterThan(0); + expect(decodeJwtPayload(refreshed.token).sub).toBe(owner.id); + expect(decodeJwtPayload(refreshed.token).aud).toBe(resourceUrl); + } + }); + + // Rotation deletes only the refresh row, so a message already in flight with the + // previous access token must still be accepted. + test('leaves the previous access token usable', async () => { + const resourceUrl = await createProtectedFormWorkflow(); + const granted = await grantFor(resourceUrl); + + await flow.refreshVirtualClientToken(granted.refreshToken, resourceUrl); + + await expect( + tokenService.verifyOAuthAccessToken(granted.token, resourceUrl), + ).resolves.toMatchObject({ user: { id: owner.id } }); + }); + + // The loser of a concurrent rotation presents a token `deleteValidByToken` already + // consumed. It must surface on the union, never as a silent reuse. + test('refuses a refresh token that was already consumed', async () => { + const resourceUrl = await createProtectedFormWorkflow(); + const granted = await grantFor(resourceUrl); + + const first = await flow.refreshVirtualClientToken(granted.refreshToken, resourceUrl); + expect(first.valid).toBe(true); + + const replay = await flow.refreshVirtualClientToken(granted.refreshToken, resourceUrl); + + expect(replay).toEqual({ valid: false, reason: 'invalid_grant' }); + }); + + test('refuses an unknown refresh token', async () => { + const resourceUrl = await createProtectedFormWorkflow(); + await grantFor(resourceUrl); + + const result = await flow.refreshVirtualClientToken('not-a-refresh-token', resourceUrl); + + expect(result).toEqual({ valid: false, reason: 'invalid_grant' }); + }); + + // The grant's own resource bounds every later token request on it (RFC 8707 §2.2). + test('refuses a refresh token presented against another resource', async () => { + const resourceUrlA = await createProtectedFormWorkflow(); + const resourceUrlB = await createProtectedFormWorkflow(); + const granted = await grantFor(resourceUrlA); + + const result = await flow.refreshVirtualClientToken(granted.refreshToken, resourceUrlB); + + expect(result).toEqual({ valid: false, reason: 'invalid_grant' }); + }); + + test('refuses a resource URL that is not a first-party protected resource', async () => { + await expect( + flow.refreshVirtualClientToken('any-token', resourceUrlFor(randomUUID())), + ).rejects.toThrow(UserError); + }); + + test('rotates a chat grant the same way', async () => { + const chatResourceUrl = await createProtectedChatWorkflow(); + const granted = await grantFor(chatResourceUrl, member.id); + + const refreshed = await flow.refreshVirtualClientToken(granted.refreshToken, chatResourceUrl); + + expect(refreshed.valid).toBe(true); + if (refreshed.valid) { + expect(decodeJwtPayload(refreshed.token).sub).toBe(member.id); + expect(decodeJwtPayload(refreshed.token).aud).toBe(chatResourceUrl); + } + }); + + // A page that refreshes for hours must not accumulate live refresh rows: the AS + // deletes the one it consumes, so the grant always holds exactly one. + test('keeps exactly one refresh row across repeated rotations', async () => { + const resourceUrl = await createProtectedChatWorkflow(); + let current = (await grantFor(resourceUrl)).refreshToken; + + for (let i = 0; i < 3; i++) { + const refreshed = await flow.refreshVirtualClientToken(current, resourceUrl); + if (!refreshed.valid) throw new Error(`rotation ${i} failed: ${refreshed.reason}`); + current = refreshed.refreshToken; + } + + await expect( + Container.get(RefreshTokenRepository).countBy({ clientId: resourceUrl }), + ).resolves.toBe(1); + }); +}); + /** * Direct cover for "a credential-connect OAuth request initiated from a chat session is * accepted as legitimate": otherwise it only holds transitively through `N8NIdentifier`. 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 22ee959df6f..0cfef2671f2 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 @@ -223,7 +223,9 @@ describe('OAuthServerService', () => { id: FIRST_PARTY_URL, name: 'My Form', redirectUris: [FIRST_PARTY_URL], - grantTypes: ['authorization_code'], + // `refresh_token` too: the AS issues one on every code exchange, and a + // long-lived trigger page rotates it instead of redirecting again. + grantTypes: ['authorization_code', 'refresh_token'], tokenEndpointAuthMethod: 'none', clientSecret: null, clientSecretExpiresAt: null, @@ -235,7 +237,7 @@ describe('OAuthServerService', () => { client_id: FIRST_PARTY_URL, client_name: 'My Form', redirect_uris: [FIRST_PARTY_URL], - grant_types: ['authorization_code'], + grant_types: ['authorization_code', 'refresh_token'], token_endpoint_auth_method: 'none', response_types: ['code'], logo_uri: undefined, @@ -253,7 +255,7 @@ describe('OAuthServerService', () => { id: CHAT_FIRST_PARTY_URL, name: 'My Chat', redirectUris: [CHAT_FIRST_PARTY_URL], - grantTypes: ['authorization_code'], + grantTypes: ['authorization_code', 'refresh_token'], tokenEndpointAuthMethod: 'none', clientSecret: null, clientSecretExpiresAt: null, diff --git a/packages/cli/src/modules/oauth-server/oauth-flow.service.ts b/packages/cli/src/modules/oauth-server/oauth-flow.service.ts index 229959f518d..fc6fdcd2513 100644 --- a/packages/cli/src/modules/oauth-server/oauth-flow.service.ts +++ b/packages/cli/src/modules/oauth-server/oauth-flow.service.ts @@ -1,7 +1,10 @@ -import { InvalidGrantError } from '@modelcontextprotocol/sdk/server/auth/errors.js'; +import { + InvalidGrantError, + InvalidTargetError, +} from '@modelcontextprotocol/sdk/server/auth/errors.js'; import { Time } from '@n8n/constants'; import { Service } from '@n8n/di'; -import { UserError, type N8nOAuth2FlowResult } from 'n8n-workflow'; +import { UserError, type N8nOAuth2FlowResult, type N8nOAuth2RefreshResult } from 'n8n-workflow'; import { createHash, randomBytes } from 'node:crypto'; import pkceChallenge from 'pkce-challenge'; @@ -100,9 +103,17 @@ export class OAuth2FlowService implements N8nOAuth2Flow { if (!result.user) { return { valid: false, reason: result.context?.reason ?? 'invalid_token' }; } + // Both are optional in the SDK's token shape but always set by our AS. Treat a + // missing one as a failed grant rather than throwing: a caller that can't refresh + // would silently stop working an hour later instead of restarting the flow now. + if (!tokens.refresh_token || tokens.expires_in === undefined) { + return { valid: false, reason: 'invalid_grant' }; + } return { valid: true, token: tokens.access_token, + refreshToken: tokens.refresh_token, + expiresIn: tokens.expires_in, user: { id: result.user.id, email: result.user.email, @@ -122,4 +133,53 @@ export class OAuth2FlowService implements N8nOAuth2Flow { throw error; } } + + /** + * Trade a refresh token from a completed flow for a fresh pair on the same grant. + * The client comes from `resourceUrl` alone, so this serves virtual clients only — the + * first-party trigger resources where client_id = redirect_uri = resource. A registered + * (DCR) client cannot refresh here; it uses the public `/oauth/token` endpoint. + * The AS rotates: the returned refresh token replaces the one passed in, and the + * old access token stays valid until its own expiry, so requests already in flight + * survive the rotation. + */ + async refreshVirtualClientToken( + refreshToken: string, + resourceUrl: string, + ): Promise { + const resource = await this.resourceRegistry.getByResourceUrl(resourceUrl); + if (!resource?.isFirstParty) { + throw new UserError(`Not a first-party protected resource: ${resourceUrl}`); + } + + const client = await this.oauthServerService.clientsStore.getClient(resourceUrl); + if (!client) return { valid: false, reason: 'invalid_client' }; + + try { + const tokens = await this.oauthServerService.exchangeRefreshToken( + client, + refreshToken, + undefined, + new URL(resourceUrl), + ); + if (!tokens.refresh_token || tokens.expires_in === undefined) { + return { valid: false, reason: 'invalid_grant' }; + } + return { + valid: true, + token: tokens.access_token, + refreshToken: tokens.refresh_token, + expiresIn: tokens.expires_in, + }; + } catch (error) { + // A token already consumed by a concurrent refresh loses the atomic + // `deleteValidByToken` race and arrives here; so does a request naming a + // resource outside the grant. Both are the caller's cue to restart the flow, + // not a server fault — surface them on the union, never as a silent reuse. + if (error instanceof InvalidGrantError || error instanceof InvalidTargetError) { + return { valid: false, reason: 'invalid_grant' }; + } + throw error; + } + } } 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 29af785c38b..647e3ad5de1 100644 --- a/packages/cli/src/modules/oauth-server/oauth-server.service.ts +++ b/packages/cli/src/modules/oauth-server/oauth-server.service.ts @@ -271,7 +271,10 @@ export class OAuthServerService implements OAuthServerProvider { id: clientId, name: resource.displayName ?? clientId, redirectUris: [clientId], - grantTypes: ['authorization_code'], + // `refresh_token` so the column matches what the grant actually supports: + // the AS issues a refresh token on every authorization-code exchange, and + // long-lived trigger pages rotate it rather than redirect again. + grantTypes: ['authorization_code', 'refresh_token'], tokenEndpointAuthMethod: 'none', clientSecret: null, clientSecretExpiresAt: null, @@ -284,7 +287,7 @@ export class OAuthServerService implements OAuthServerProvider { client_id: clientId, client_name: resource.displayName ?? clientId, redirect_uris: [clientId], - grant_types: ['authorization_code'], + grant_types: ['authorization_code', 'refresh_token'], token_endpoint_auth_method: 'none', response_types: ['code'], logo_uri: undefined, diff --git a/packages/cli/src/services/oauth2-flow-proxy.service.ts b/packages/cli/src/services/oauth2-flow-proxy.service.ts index 9fc04a7fd40..2e72f4440ec 100644 --- a/packages/cli/src/services/oauth2-flow-proxy.service.ts +++ b/packages/cli/src/services/oauth2-flow-proxy.service.ts @@ -1,9 +1,23 @@ import { Service } from '@n8n/di'; -import { UnexpectedError, type N8nOAuth2FlowResult } from 'n8n-workflow'; +import { + UnexpectedError, + type N8nOAuth2FlowResult, + type N8nOAuth2RefreshResult, +} from 'n8n-workflow'; +/** + * The in-process OAuth2 flow for first-party trigger resources. Every method resolves a + * virtual client from the resource URL, where client_id = redirect_uri = the trigger URL. + * A registered (DCR) client cannot use these methods — it goes through the public + * `/oauth/token` endpoint instead. + */ export interface N8nOAuth2Flow { begin(resourceUrl: string, metadata?: Record): Promise; complete(code: string, state: string): Promise; + refreshVirtualClientToken( + refreshToken: string, + resourceUrl: string, + ): Promise; } @Service() @@ -15,12 +29,20 @@ export class OAuth2FlowProxy implements N8nOAuth2Flow { } async begin(resourceUrl: string, metadata?: Record): Promise { - if (!this.provider) throw new UnexpectedError('OAuth2 form flow is not available'); + if (!this.provider) throw new UnexpectedError('OAuth2 trigger flow is not available'); return await this.provider.begin(resourceUrl, metadata); } async complete(code: string, state: string): Promise { - if (!this.provider) throw new UnexpectedError('OAuth2 form flow is not available'); + if (!this.provider) throw new UnexpectedError('OAuth2 trigger flow is not available'); return await this.provider.complete(code, state); } + + async refreshVirtualClientToken( + refreshToken: string, + resourceUrl: string, + ): Promise { + if (!this.provider) throw new UnexpectedError('OAuth2 trigger flow is not available'); + return await this.provider.refreshVirtualClientToken(refreshToken, resourceUrl); + } } diff --git a/packages/cli/src/webhooks/__tests__/webhook-request-sanitizer.test.ts b/packages/cli/src/webhooks/__tests__/webhook-request-sanitizer.test.ts index 5c91d4a74c5..bcb5bf20722 100644 --- a/packages/cli/src/webhooks/__tests__/webhook-request-sanitizer.test.ts +++ b/packages/cli/src/webhooks/__tests__/webhook-request-sanitizer.test.ts @@ -321,13 +321,15 @@ describe('webhookRequestSanitizer', () => { it('should remove every disallowed cookie in one pass', () => { mockRequest.headers = { cookie: - 'n8n-auth=a; n8n-browserId=b; n8n-form-auth-ex-12345=c; n8n-form-oauth=d; other-cookie=value', + 'n8n-auth=a; n8n-browserId=b; n8n-form-auth-ex-12345=c; n8n-form-oauth=d; n8n-chat-oauth=e; n8n-chat-oauth-refresh=f; other-cookie=value', }; mockRequest.cookies = { 'n8n-auth': 'a', 'n8n-browserId': 'b', 'n8n-form-auth-ex-12345': 'c', 'n8n-form-oauth': 'd', + 'n8n-chat-oauth': 'e', + 'n8n-chat-oauth-refresh': 'f', 'other-cookie': 'value', }; @@ -363,4 +365,34 @@ describe('webhookRequestSanitizer', () => { expect(mockRequest.cookies).toEqual({ 'other-cookie': 'value' }); }); }); + + // The chat endpoints skip sanitizing for their own node type, so the hosted page + // still receives these. Every other webhook must not see them — the `-refresh` one + // carries a 30-day credential. + describe('when the chat cookies are present', () => { + const chatCookieNames = ['n8n-chat-oauth', 'n8n-chat-oauth-refresh']; + + it.each(chatCookieNames)('should remove %s from the header', (name) => { + mockRequest.headers = { + cookie: `${name}=abc123; other-cookie=value`, + }; + + sanitizeWebhookRequest(mockRequest); + + expect(mockRequest.headers.cookie).toBe('other-cookie=value'); + }); + + it.each(chatCookieNames)('should remove %s from parsed cookies', (name) => { + mockRequest.cookies = { + [name]: 'abc123', + 'other-cookie': 'value', + }; + + sanitizeWebhookRequest(mockRequest); + + expect(mockRequest.cookies).toEqual({ + 'other-cookie': 'value', + }); + }); + }); }); diff --git a/packages/cli/src/webhooks/webhook-helpers.ts b/packages/cli/src/webhooks/webhook-helpers.ts index 647cdc532e2..36e6f6a03ea 100644 --- a/packages/cli/src/webhooks/webhook-helpers.ts +++ b/packages/cli/src/webhooks/webhook-helpers.ts @@ -619,6 +619,9 @@ export async function executeWebhook( additionalData.completeN8nOAuth2Flow = async (code: string, state: string) => await Container.get(OAuth2FlowProxy).complete(code, state); + additionalData.refreshN8nOAuth2Flow = async (refreshToken: string, resourceUrl: string) => + await Container.get(OAuth2FlowProxy).refreshVirtualClientToken(refreshToken, resourceUrl); + // Captured here so `establishTriggerIdentity` seals the gate that admitted this // request, instead of resolving the resource a second time. let admittedBy: { resource: string; grant?: OAuthResourceGrant } | undefined; diff --git a/packages/cli/src/webhooks/webhook-request-sanitizer.ts b/packages/cli/src/webhooks/webhook-request-sanitizer.ts index f877050fd1d..33d4de8f967 100644 --- a/packages/cli/src/webhooks/webhook-request-sanitizer.ts +++ b/packages/cli/src/webhooks/webhook-request-sanitizer.ts @@ -2,6 +2,8 @@ import type { Request } from 'express'; import { AUTH_COOKIE_NAME, + CHAT_OAUTH_COOKIE_NAME, + CHAT_OAUTH_REFRESH_COOKIE_NAME, FORM_AUTH_COOKIE_PREFIX, FORM_OAUTH_COOKIE_NAME, OIDC_NONCE_COOKIE_NAME, @@ -15,11 +17,12 @@ const BROWSER_ID_COOKIE_NAME = 'n8n-browserId'; /** * Cookies n8n issues for its own UI and sign-in flows. They are set without a `path`, so - * browsers send them to `/webhook/*` too. + * browsers send them to `/webhook/*` too. Hence an explicit list of names rather than an + * `n8n-` prefix rule, which would take unrelated cookies with it. * - * `n8n-form-oauth` is excluded: the form OAuth2 flow reads it back off the raw header on the - * redirect hop, so it has to keep flowing. Hence an explicit list of names rather than an - * `n8n-` prefix rule, which would take that cookie with it. + * The form and chat cookies belong to their own endpoints, which skip sanitizing entirely + * for their own node types (see `authAllowlistedNodes`), so those pages still receive + * them. Every other webhook has no use for them. */ const DISALLOWED_COOKIES = new Set([ AUTH_COOKIE_NAME, @@ -30,6 +33,8 @@ const DISALLOWED_COOKIES = new Set([ OIDC_STATE_COOKIE_NAME, OIDC_NONCE_COOKIE_NAME, FORM_OAUTH_COOKIE_NAME, + CHAT_OAUTH_COOKIE_NAME, + CHAT_OAUTH_REFRESH_COOKIE_NAME, ]); // The form auth cookie's name appends the workflow or execution it was minted diff --git a/packages/core/src/execution-engine/node-execution-context/webhook-context.ts b/packages/core/src/execution-engine/node-execution-context/webhook-context.ts index e747eb441f0..1f0dfdabbfa 100644 --- a/packages/core/src/execution-engine/node-execution-context/webhook-context.ts +++ b/packages/core/src/execution-engine/node-execution-context/webhook-context.ts @@ -20,6 +20,7 @@ import type { Workflow, WorkflowExecuteMode, N8nOAuth2FlowResult, + N8nOAuth2RefreshResult, } from 'n8n-workflow'; import { UnexpectedError, createEmptyRunExecutionData } from 'n8n-workflow'; @@ -207,6 +208,16 @@ export class WebhookContext extends NodeExecutionContext implements IWebhookFunc return await this.additionalData.completeN8nOAuth2Flow(code, state); } + async refreshN8nOAuth2Flow( + refreshToken: string, + resourceUrl: string, + ): Promise { + if (!this.additionalData.refreshN8nOAuth2Flow) { + throw new UnexpectedError('OAuth2 flow is not available'); + } + return await this.additionalData.refreshN8nOAuth2Flow(refreshToken, resourceUrl); + } + async validateN8nOAuth2Token( token: string, resourceUrl: string, diff --git a/packages/nodes-base/nodes/Form/test/utils.test.ts b/packages/nodes-base/nodes/Form/test/utils.test.ts index 64d1a9803ce..9a818cc108b 100644 --- a/packages/nodes-base/nodes/Form/test/utils.test.ts +++ b/packages/nodes-base/nodes/Form/test/utils.test.ts @@ -991,6 +991,8 @@ describe('FormTrigger, formWebhook', () => { ctx.completeN8nOAuth2Flow.mockResolvedValue({ valid: true, token: 'as-token', + refreshToken: 'refresh-token', + expiresIn: 3600, user: authedUser, }); @@ -1053,6 +1055,8 @@ describe('FormTrigger, formWebhook', () => { ctx.completeN8nOAuth2Flow.mockResolvedValue({ valid: true, token: 'as-token', + refreshToken: 'refresh-token', + expiresIn: 3600, user: authedUser, metadata: { query: 'foo=bar' }, }); diff --git a/packages/workflow/src/interfaces.ts b/packages/workflow/src/interfaces.ts index 9e026c19a07..ba22b127b60 100644 --- a/packages/workflow/src/interfaces.ts +++ b/packages/workflow/src/interfaces.ts @@ -155,7 +155,26 @@ export type N8nOAuth2ValidationResult = | { valid: false; reason: OAuth2FailureReason }; export type N8nOAuth2FlowResult = - | { valid: true; token: string; user: IUser; metadata?: Record } + | { + valid: true; + token: string; + /** Rotated on every use. Never hand this to a browser document or to page script. */ + refreshToken: string; + /** Lifetime of `token` in seconds, as the AS reports it. A duration, not an + * absolute `exp`, so a browser scheduling off it is immune to clock skew. */ + expiresIn: number; + user: IUser; + metadata?: Record; + } + | { valid: false; reason: string }; + +/** + * Result of trading a refresh token for a fresh pair. Carries no user: the grant + * the token belongs to already fixes the subject, and the caller re-validates the + * access token when it needs the identity. + */ +export type N8nOAuth2RefreshResult = + | { valid: true; token: string; refreshToken: string; expiresIn: number } | { valid: false; reason: string }; export type ProjectSharingData = { @@ -1534,6 +1553,14 @@ export interface IWebhookFunctions extends FunctionsBaseWithRequiredKeys<'getMod * success, or a failure reason. */ completeN8nOAuth2Flow(code: string, state: string): Promise; + /** + * Trades the refresh token from a completed flow for a fresh access/refresh pair on + * the same grant, keeping a long-lived page working past the access token's one-hour + * life without a new redirect. The AS rotates the refresh token, so the caller must + * store the returned one and drop the old one. `resourceUrl` must name the resource + * the grant was approved for. + */ + refreshN8nOAuth2Flow(refreshToken: string, resourceUrl: string): Promise; /** * Verifies an AS access token against `resourceUrl` (the expected audience) without * running a redirect flow. Used by resource-server triggers (MCP) that receive a @@ -3760,6 +3787,10 @@ export interface IWorkflowExecuteAdditionalData { */ beginN8nOAuth2Flow?: (resourceUrl: string, metadata?: Record) => Promise; completeN8nOAuth2Flow?: (code: string, state: string) => Promise; + refreshN8nOAuth2Flow?: ( + refreshToken: string, + resourceUrl: string, + ) => Promise; validateN8nOAuth2Token?: ( token: string, resourceUrl: string,