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 2cd693c7cd2..7142ea1ce51 100644 --- a/packages/@n8n/nodes-langchain/nodes/trigger/ChatTrigger/ChatTrigger.node.ts +++ b/packages/@n8n/nodes-langchain/nodes/trigger/ChatTrigger/ChatTrigger.node.ts @@ -21,7 +21,6 @@ import type { INodeExecutionData, IBinaryData, INodeProperties, - IUser, } from 'n8n-workflow'; import * as a from 'node:assert'; import { ChatTriggerConfig } from '@n8n/config'; @@ -34,15 +33,13 @@ import { validateAuth, } from './GenericFunctions'; import { - buildAbsoluteChatUrl, buildInnerFrameSrc, CHAT_FRAME_SANDBOX, isChatOAuth2Enabled, isShellInnerRequest, - readAuthCookie, } from './shell'; import { createPage, createShellPage } from './templates'; -import { assertValidLoadPreviousSessionOption } from './types'; +import { assertValidLoadPreviousSessionOption, type ChatFrameIdentity } from './types'; const isPublicChatTriggerDisabled = () => Container.get(ChatTriggerConfig).disablePublicChat; const allowFileUploadsOption: INodeProperties = { @@ -921,37 +918,19 @@ export class ChatTrigger extends Node { // An n8n-controlled shell on the real origin, with the author's chat in a frame // that has no origin. The connect experience needs the real origin (OAuth popup, // success channel, `localStorage`), so nothing author-shaped may live there. - let shellInner = false; - let authToken: string | undefined; - let visitor: IUser | undefined; + let frameIdentity: ChatFrameIdentity | undefined; if (isChatOAuth2Enabled() && authentication === 'n8nUserAuth') { - shellInner = isShellInnerRequest(req); - const resourceUrl = ctx.getWebhookResourceUrl('default'); if (!resourceUrl) { throw new NodeOperationError(ctx.getNode(), 'Default webhook url not set'); } - if (!shellInner) { - // Outer shell: gates page access on the live session, then runs the AS - // handshake here — a normal top-level document with real cookies, unlike - // the sandboxed, opaque-origin frame this shell is about to create. - const authCookie = readAuthCookie(req); - if (authCookie) { - try { - visitor = await ctx.validateCookieAuth(authCookie); - } catch {} - } - - if (!visitor) { - res.writeHead(302, { - Location: `/signin?redirect=${encodeURIComponent(buildAbsoluteChatUrl(req))}`, - }); - res.end(); - 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) { return { noWebhookResponse: true }; @@ -975,8 +954,7 @@ export class ChatTrigger extends Node { res.end(); return { noWebhookResponse: true }; } - visitor = identity.visitor; - authToken = identity.authToken; + frameIdentity = identity; // By header as well as by the iframe's attribute, so the document has no // origin even if the attribute is ever stripped. @@ -998,9 +976,7 @@ export class ChatTrigger extends Node { allowedFilesMimeTypes: options.allowedFilesMimeTypes, customCss: options.customCss, enableStreaming, - shellInner, - authToken, - visitor, + frameIdentity, }); res.status(200).send(page).end(); diff --git a/packages/@n8n/nodes-langchain/nodes/trigger/ChatTrigger/GenericFunctions.ts b/packages/@n8n/nodes-langchain/nodes/trigger/ChatTrigger/GenericFunctions.ts index 48273576702..12fa9508651 100644 --- a/packages/@n8n/nodes-langchain/nodes/trigger/ChatTrigger/GenericFunctions.ts +++ b/packages/@n8n/nodes-langchain/nodes/trigger/ChatTrigger/GenericFunctions.ts @@ -1,6 +1,6 @@ import basicAuth from 'basic-auth'; import { UnexpectedError } from 'n8n-workflow'; -import type { ICredentialDataDecryptedObject, IUser, IWebhookFunctions } from 'n8n-workflow'; +import type { ICredentialDataDecryptedObject, IWebhookFunctions } from 'n8n-workflow'; import { ChatTriggerAuthorizationError } from './error'; import { @@ -9,7 +9,7 @@ import { readChatOAuthToken, setChatOAuthToken, } from './shell'; -import type { AuthenticationChatOption } from './types'; +import type { AuthenticationChatOption, ChatFrameIdentity } from './types'; export async function validateAuth(context: IWebhookFunctions) { const authentication = context.getNodeParameter( @@ -185,7 +185,7 @@ export async function establishChatSessionIdentity( export async function resolveInnerFrameIdentity( context: IWebhookFunctions, resourceUrl: string, -): Promise<{ visitor: IUser; authToken: string } | null> { +): Promise { const req = context.getRequestObject(); const res = context.getResponseObject(); 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 ac6370229a9..2cdded3d014 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 @@ -423,12 +423,20 @@ describe('ChatTrigger Node', () => { const renderedPage = () => vi.mocked(mockResponse.send).mock.calls.at(-1)?.[0] as string; + // Every body and header this request produced, so a `/signin` anywhere in the + // response — a redirect Location as much as a rendered page — shows up. + const everySentResponse = () => + JSON.stringify([ + vi.mocked(mockResponse.send).mock.calls, + vi.mocked(mockResponse.writeHead).mock.calls, + vi.mocked(mockResponse.setHeader).mock.calls, + ]); + beforeEach(() => { mockContext.getWebhookName.mockReturnValue('setup'); mockContext.getNodeWebhookUrl.mockReturnValue('http://localhost:5678/webhook/abc/chat'); mockContext.getWebhookResourceUrl.mockReturnValue('http://localhost:5678/webhook/abc/chat'); mockContext.getInstanceId.mockReturnValue('instance-1'); - mockContext.validateCookieAuth.mockResolvedValue(visitor); mockContext.getNode.mockReturnValue({ id: 'node-1', name: 'Chat Trigger', @@ -445,7 +453,6 @@ describe('ChatTrigger Node', () => { mockRequest.headers = { 'x-forwarded-proto': 'http', host: 'localhost:5678', - cookie: 'n8n-auth=session-token', }; mockRequest.query = {}; mockRequest.originalUrl = '/webhook/abc/chat'; @@ -478,7 +485,6 @@ describe('ChatTrigger Node', () => { mockRequest.headers = { 'x-forwarded-proto': 'http', host: 'localhost:5678', - cookie: 'n8n-auth=session-token', 'sec-fetch-dest': 'iframe', }; @@ -512,7 +518,6 @@ describe('ChatTrigger Node', () => { mockRequest.headers = { 'x-forwarded-proto': 'http', host: 'localhost:5678', - cookie: 'n8n-auth=session-token', 'sec-fetch-dest': 'iframe', }; vi.mocked(resolveInnerFrameIdentity).mockResolvedValue(null); @@ -531,7 +536,6 @@ describe('ChatTrigger Node', () => { mockRequest.headers = { 'x-forwarded-proto': 'http', host: 'localhost:5678', - cookie: 'n8n-auth=session-token', 'sec-fetch-dest': 'document', }; @@ -541,17 +545,21 @@ describe('ChatTrigger Node', () => { expect(renderedPage()).not.toContain('createChat'); }); - it('sends an unauthenticated visitor to sign in', async () => { + // The page used to bounce a visitor with no editor session to `/signin` before the + // AS ever saw them, which defeated the whole point of end-user credentials for + // external visitors. The flow authenticates them instead. + it('begins the OAuth2 flow for a visitor with no session', async () => { mockRequest.headers = { 'x-forwarded-proto': 'http', host: 'localhost:5678' }; - mockContext.validateCookieAuth.mockRejectedValue(new Error('nope')); const result = await renderSetupPage(); expect(result).toEqual({ noWebhookResponse: true }); - expect(mockResponse.writeHead).toHaveBeenCalledWith(302, { - Location: '/signin?redirect=http%3A%2F%2Flocalhost%3A5678%2Fwebhook%2Fabc%2Fchat', - }); - expect(mockResponse.send).not.toHaveBeenCalled(); + expect(establishChatSessionIdentity).toHaveBeenCalledWith( + mockContext, + 'http://localhost:5678/webhook/abc/chat', + ); + expect(mockContext.validateCookieAuth).not.toHaveBeenCalled(); + expect(everySentResponse()).not.toContain('/signin'); }); it('renders the page unsplit when the flag is off', async () => { @@ -560,6 +568,7 @@ describe('ChatTrigger Node', () => { await renderSetupPage(); expect(mockResponse.setHeader).not.toHaveBeenCalled(); + expect(establishChatSessionIdentity).not.toHaveBeenCalled(); expect(renderedPage()).toContain('createChat'); expect(renderedPage()).not.toContain('n8nShellInner'); }); @@ -570,6 +579,7 @@ describe('ChatTrigger Node', () => { await renderSetupPage(authentication); expect(mockResponse.setHeader).not.toHaveBeenCalled(); + expect(establishChatSessionIdentity).not.toHaveBeenCalled(); expect(renderedPage()).toContain('createChat'); expect(renderedPage()).not.toContain('n8nShellInner'); }, 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 1048c819e7a..e5ba1473083 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,10 @@ import type { Request, Response } from 'express'; import { - buildAbsoluteChatUrl, buildInnerFrameSrc, clearChatOAuthToken, isChatOAuth2Enabled, isShellInnerRequest, - readAuthCookie, readChatOAuthToken, setChatOAuthToken, } from '../shell'; @@ -90,42 +88,6 @@ describe('buildInnerFrameSrc', () => { }); }); -describe('buildAbsoluteChatUrl', () => { - it('prefers the forwarding headers over the request', () => { - const req = request({ - headers: { 'x-forwarded-proto': 'https', 'x-forwarded-host': 'chat.example.com' }, - }); - - expect(buildAbsoluteChatUrl(req)).toBe('https://chat.example.com/webhook/abc/chat'); - }); - - it('falls back to the request protocol and Host', () => { - const req = request({ headers: { host: 'localhost:5678' } }); - - expect(buildAbsoluteChatUrl(req)).toBe('http://localhost:5678/webhook/abc/chat'); - }); -}); - -describe('readAuthCookie', () => { - it('reads the session cookie from the raw header', () => { - const req = request({ headers: { cookie: 'other=1; n8n-auth=token-value; more=2' } }); - - expect(readAuthCookie(req)).toBe('token-value'); - }); - - it('returns null when the cookie is absent', () => { - expect(readAuthCookie(request({ headers: { cookie: 'other=1' } }))).toBeNull(); - expect(readAuthCookie(request())).toBeNull(); - }); - - // `n8n-auth` must not be matched inside a longer cookie name. - it('does not match a cookie whose name merely ends with n8n-auth', () => { - const req = request({ headers: { cookie: 'notn8n-auth=nope' } }); - - expect(readAuthCookie(req)).toBeNull(); - }); -}); - describe('chat OAuth2 one-hop cookie', () => { const resourceUrl = 'http://localhost:5678/webhook/abc/chat'; 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 5c589081175..4f1567869c8 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 @@ -683,9 +683,7 @@ describe('createPage inside the shell frame', () => { const inner = createPage({ ...params, - shellInner: true, - authToken: 'signed.jwt.token', - visitor, + frameIdentity: { visitor, authToken: 'signed.jwt.token' }, }); it('stands in for localStorage before the widget loads', () => { @@ -707,12 +705,13 @@ describe('createPage inside the shell frame', () => { expect(inner).toContain('\'x-auth-token\': "signed.jwt.token",'); }); + // Not merely skipped at runtime: the bootstrap is never emitted, so there is no + // path from this document to a login endpoint it couldn't reach or a sign-in page it + // couldn't render. it('takes the visitor from the server instead of fetching the login endpoint', () => { - expect(inner).toContain('const injectedVisitor = {"id":"user-1"'); - expect(inner).toContain('if (injectedVisitor) {'); - expect(inner.indexOf('if (injectedVisitor) {')).toBeLessThan( - inner.indexOf("fetch('/rest/login'"), - ); + expect(inner).toContain('const metadata = { user: {"id":"user-1"'); + expect(inner).not.toContain("fetch('/rest/login'"); + expect(inner).not.toContain("'/signin?redirect='"); }); it('still renders the author own styling', () => { @@ -729,12 +728,25 @@ describe('createPage inside the shell frame', () => { expect(plain).toContain('const injectedVisitor = null;'); }); - // The token and the visitor only ever belong to the sandboxed render. - it('ignores a token or visitor passed without the inner flag', () => { - const stray = createPage({ ...params, authToken: 'signed.jwt.token', visitor }); - - expect(stray).toContain('const injectedVisitor = null;'); - expect(stray).not.toContain('x-auth-token'); + // 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'"); + expect(plain).toContain("'/signin?redirect='"); }); }); + + // A frame render holding half an identity would silently serve an anonymous chat where + // the single-document path redirects to sign-in, and the frame can resolve neither half + // for itself. Both fields are required together, so that state can't be expressed — + // this fails the build rather than the run if the shape ever loosens. + it('cannot represent a frame render missing half its identity', () => { + type FrameIdentity = Parameters[0]['frameIdentity']; + + // @ts-expect-error the visitor and their token only ever travel together + const withoutVisitor: FrameIdentity = { authToken: 'signed.jwt.token' }; + // @ts-expect-error ...in both directions + const withoutToken: FrameIdentity = { visitor }; + + expect([withoutVisitor, withoutToken]).toHaveLength(2); + }); }); diff --git a/packages/@n8n/nodes-langchain/nodes/trigger/ChatTrigger/shell.ts b/packages/@n8n/nodes-langchain/nodes/trigger/ChatTrigger/shell.ts index 4bbcf6bf001..0380ceed619 100644 --- a/packages/@n8n/nodes-langchain/nodes/trigger/ChatTrigger/shell.ts +++ b/packages/@n8n/nodes-langchain/nodes/trigger/ChatTrigger/shell.ts @@ -36,26 +36,6 @@ export function buildInnerFrameSrc(req: Request): string { return `${path}?${params.toString()}`; } -/** - * For the post-signin redirect, from the forwarding headers so it survives a proxy. Only - * ever points back at this page, so a spoofed Host breaks one visitor's return trip at - * worst — it can't redirect to another origin. - */ -export function buildAbsoluteChatUrl(req: Request): string { - const headerValue = (name: string) => { - const raw = req.headers[name]; - return typeof raw === 'string' ? raw.trim() : undefined; - }; - const protocol = headerValue('x-forwarded-proto') ?? req.protocol ?? 'http'; - const host = headerValue('x-forwarded-host') ?? req.headers.host ?? ''; - return `${protocol}://${host}${req.originalUrl}`; -} - -export function readAuthCookie(req: Request): string | null { - const match = (req.headers.cookie ?? '').match(/(?:^|;\s*)n8n-auth=([^;]+)/); - return match ? match[1].trim() : null; -} - // 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 diff --git a/packages/@n8n/nodes-langchain/nodes/trigger/ChatTrigger/templates.ts b/packages/@n8n/nodes-langchain/nodes/trigger/ChatTrigger/templates.ts index 94708a8ad23..5da20b3d537 100644 --- a/packages/@n8n/nodes-langchain/nodes/trigger/ChatTrigger/templates.ts +++ b/packages/@n8n/nodes-langchain/nodes/trigger/ChatTrigger/templates.ts @@ -1,8 +1,11 @@ -import type { IUser } from 'n8n-workflow'; import sanitizeHtml from 'sanitize-html'; import { CHAT_FRAME_SANDBOX } from './shell'; -import type { AuthenticationChatOption, LoadPreviousSessionChatOption } from './types'; +import type { + AuthenticationChatOption, + ChatFrameIdentity, + LoadPreviousSessionChatOption, +} from './types'; function sanitizeUserInput(input: unknown): string { // Only strings and numbers are meaningful display values; sanitize-html @@ -172,9 +175,7 @@ export function createPage({ allowedFilesMimeTypes, customCss, enableStreaming, - shellInner, - authToken, - visitor, + frameIdentity, }: { instanceId: string; webhookUrl?: string; @@ -190,12 +191,12 @@ export function createPage({ allowedFilesMimeTypes?: string; customCss?: string; enableStreaming?: boolean; - /** True when this page renders inside the shell's sandboxed frame. */ - shellInner?: boolean; - /** Sent as `x-auth-token` on every message, since the frame can't send cookies. */ - authToken?: string; - /** Injected server-side: the frame can't fetch `/rest/login` for itself. */ - visitor?: IUser; + /** + * Set only for the render inside the shell's sandboxed frame, carrying the identity the + * server resolved for it. Absent means the single-document render, which resolves its + * own identity in the browser (or has none, under `none`/`basicAuth`). + */ + frameIdentity?: ChatFrameIdentity; }) { const validAuthenticationOptions: AuthenticationChatOption[] = [ 'none', @@ -225,43 +226,18 @@ export function createPage({ const sanitizedInitialMessages = getSanitizedInitialMessages(initialMessages); const sanitizedI18nConfig = getSanitizedI18nConfig(en || {}); - // Inner render only, where the frame's opaque origin makes the `/rest/login` bootstrap - // below impossible. Field-by-field so nothing else on the user object reaches the page. - const injectedVisitor = - shellInner && visitor - ? escapeForScriptContext({ - id: visitor.id, - firstName: visitor.firstName, - lastName: visitor.lastName, - email: visitor.email, - }) - : 'null'; + const shellInner = frameIdentity !== undefined; - return ` - - - - - Chat - - - - - - ${shellInner ? innerBootstrapScript : ''} -