From 3a4ea3b4d64270d6fe6f0eb0d4a83f75ba223d26 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Tue, 27 Oct 2026 15:05:35 +0100 Subject: [PATCH] fix(terminal): forward the session cookie instead of guessing its name The terminal server built the Laravel session cookie name from APP_NAME with its own regex and ignored SESSION_COOKIE. When the name differed from Laravel's, every connection closed with "Missing required tokens". Forward the browser Cookie header so Laravel reads its own session. Also remove the text rejection messages kept for terminal clients from before close codes; the server and client now ship in the same image. Closes #10549 Co-Authored-By: Claude Opus 5.5 --- docker/coolify-terminal/terminal-server.js | 43 +++++------------ docker/coolify-terminal/terminal-utils.js | 47 ++++++++++++------- .../coolify-terminal/terminal-utils.test.js | 41 +++++++++------- resources/js/terminal-connection.js | 28 ++--------- resources/js/terminal-connection.test.js | 17 +++---- resources/js/terminal.js | 12 ++--- 6 files changed, 81 insertions(+), 107 deletions(-) diff --git a/docker/coolify-terminal/terminal-server.js b/docker/coolify-terminal/terminal-server.js index 8f34970a9c..cdf327b93a 100755 --- a/docker/coolify-terminal/terminal-server.js +++ b/docker/coolify-terminal/terminal-server.js @@ -12,6 +12,7 @@ import { extractTargetHost, extractTimeout, detachPtyProcess, + getTerminalAuthHeaders, getTerminalProcessEnv, getTerminalSessionTimeout, isAttachCommand, @@ -111,32 +112,23 @@ const server = http.createServer((req, res) => { } }); -const getSessionCookie = (req) => { - const cookies = parseCookie(req.headers.cookie || ''); - const xsrfToken = cookies['XSRF-TOKEN']; - const appName = process.env.APP_NAME || 'laravel'; - const sessionCookieName = `${appName.replace(/[^a-zA-Z0-9]/g, '_').toLowerCase()}_session`; - return { - sessionCookieName, - xsrfToken: xsrfToken, - laravelSession: cookies[sessionCookieName] - } +const getAuthHeaders = (req) => { + const cookieHeader = req.headers.cookie || ''; + return getTerminalAuthHeaders(cookieHeader, parseCookie(cookieHeader)['XSRF-TOKEN']); } const verifyClient = async (req) => { - const sessionCookie = getSessionCookie(req); + const authHeaders = getAuthHeaders(req); const requestContext = { remoteAddress: req.socket?.remoteAddress, origin: req.headers.origin, - sessionCookieName: sessionCookie.sessionCookieName, - hasXsrfToken: Boolean(sessionCookie.xsrfToken), - hasLaravelSession: Boolean(sessionCookie.laravelSession), + hasAuthTokens: Boolean(authHeaders), }; logTerminal('log', 'Verifying websocket client.', requestContext); // Authenticate with Laravel backend - const result = await authenticateTerminalUpgrade(sessionCookie, postToCoolify); + const result = await authenticateTerminalUpgrade(authHeaders, postToCoolify); if (result.authenticated) { logTerminal('log', 'Websocket client authentication succeeded.', requestContext); @@ -170,7 +162,7 @@ wss.on('connection', async (ws, req) => { const userId = generateUserId(); ws.userId = userId; - const { xsrfToken, laravelSession, sessionCookieName } = getSessionCookie(req); + const authHeaders = getAuthHeaders(req); const userSession = { ws, userId, @@ -180,17 +172,12 @@ wss.on('connection', async (ws, req) => { authReady: false, pendingMessages: [], terminalSessionTimer: null, - authHeaders: { - 'Cookie': `${sessionCookieName}=${laravelSession}`, - 'X-XSRF-TOKEN': xsrfToken, - }, + authHeaders, }; const connectionContext = { userId, remoteAddress: req.socket?.remoteAddress, - sessionCookieName, - hasXsrfToken: Boolean(xsrfToken), - hasLaravelSession: Boolean(laravelSession), + hasAuthTokens: Boolean(authHeaders), }; // Register socket handlers up front so messages sent immediately by the client @@ -214,17 +201,14 @@ wss.on('connection', async (ws, req) => { }); // Verify presence of required tokens - if (!laravelSession || !xsrfToken) { + if (!authHeaders) { logTerminal('warn', 'Closing websocket connection because required auth tokens are missing.', connectionContext); rejectTerminalSocket(ws, TERMINAL_CLOSE_CODES.AUTH_REJECTED, 'Unauthorized: Missing required tokens'); return; } try { - const response = await postToCoolify('/terminal/auth/ips', { - 'Cookie': `${sessionCookieName}=${laravelSession}`, - 'X-XSRF-TOKEN': xsrfToken - }); + const response = await postToCoolify('/terminal/auth/ips', authHeaders); if (response.status !== 200) { throw createHttpError(response); @@ -464,13 +448,12 @@ async function handleCommand(ws, command, userId) { }, terminalSessionTimeout * 1000); } -// The text message is kept for clients that predate the close code. function rejectTerminalToken(userSession, message) { logTerminal('warn', 'Closing websocket connection because the terminal token was rejected.', { userId: userSession.userId, reason: message, }); - rejectTerminalSocket(userSession.ws, TERMINAL_CLOSE_CODES.TOKEN_REJECTED, message, { message }); + rejectTerminalSocket(userSession.ws, TERMINAL_CLOSE_CODES.TOKEN_REJECTED, message); } async function handleTerminalToken(userSession, token) { diff --git a/docker/coolify-terminal/terminal-utils.js b/docker/coolify-terminal/terminal-utils.js index 8675e10516..0c7afcb012 100644 --- a/docker/coolify-terminal/terminal-utils.js +++ b/docker/coolify-terminal/terminal-utils.js @@ -14,16 +14,35 @@ export const TERMINAL_CLOSE_CODES = Object.freeze({ export const TERMINAL_REJECTED_SOCKET_TERMINATE_MS = 2000; /** - * Authenticates a terminal WebSocket upgrade with the browser's Laravel session - * cookie and XSRF token. Performs the same checks as the former verifyClient - * callback; only the way a rejection is reported to the client differs. + * Builds the headers that authenticate terminal requests to Coolify. The whole + * browser Cookie header is forwarded, so Laravel finds its session cookie by + * its own name (SESSION_COOKIE or the slug of APP_NAME). * - * @param {{ laravelSession?: string, xsrfToken?: string, sessionCookieName: string }} session + * @param {string} cookieHeader + * @param {string | undefined} xsrfToken + * @returns {{ 'Cookie': string, 'X-XSRF-TOKEN': string } | null} null when a required value is missing + */ +export function getTerminalAuthHeaders(cookieHeader, xsrfToken) { + if (!cookieHeader || !xsrfToken) { + return null; + } + + return { + 'Cookie': cookieHeader, + 'X-XSRF-TOKEN': xsrfToken, + }; +} + +/** + * Authenticates a terminal WebSocket upgrade with the browser's Laravel session + * cookie and XSRF token. + * + * @param {{ 'Cookie': string, 'X-XSRF-TOKEN': string } | null} authHeaders from getTerminalAuthHeaders() * @param {(path: string, headers: object) => Promise<{ status: number }>} postToCoolify * @returns {Promise<{ authenticated: true } | { authenticated: false, closeCode: number, reason: string, status?: number, error?: Error }>} */ -export async function authenticateTerminalUpgrade({ laravelSession, xsrfToken, sessionCookieName }, postToCoolify) { - if (!laravelSession || !xsrfToken) { +export async function authenticateTerminalUpgrade(authHeaders, postToCoolify) { + if (!authHeaders) { return { authenticated: false, closeCode: TERMINAL_CLOSE_CODES.AUTH_REJECTED, @@ -32,10 +51,7 @@ export async function authenticateTerminalUpgrade({ laravelSession, xsrfToken, s } try { - const response = await postToCoolify('/terminal/auth', { - 'Cookie': `${sessionCookieName}=${laravelSession}`, - 'X-XSRF-TOKEN': xsrfToken, - }); + const response = await postToCoolify('/terminal/auth', authHeaders); if (response.status === 200) { return { authenticated: true }; @@ -58,11 +74,10 @@ export async function authenticateTerminalUpgrade({ laravelSession, xsrfToken, s } /** - * Closes a WebSocket that must not be used, optionally sending a legacy text - * message first for clients that predate close codes. The socket is terminated - * if the peer does not complete the close handshake quickly. + * Closes a WebSocket that must not be used. The socket is terminated if the + * peer does not complete the close handshake quickly. */ -export function rejectTerminalSocket(ws, closeCode, reason, { message = null, terminateAfterMs = TERMINAL_REJECTED_SOCKET_TERMINATE_MS } = {}) { +export function rejectTerminalSocket(ws, closeCode, reason, { terminateAfterMs = TERMINAL_REJECTED_SOCKET_TERMINATE_MS } = {}) { const terminateTimer = setTimeout(() => ws.terminate(), terminateAfterMs); terminateTimer.unref?.(); ws.once('close', () => clearTimeout(terminateTimer)); @@ -70,10 +85,6 @@ export function rejectTerminalSocket(ws, closeCode, reason, { message = null, te // malformed frame from an unauthenticated client) would crash the server. ws.on('error', () => ws.terminate()); - if (message !== null) { - ws.send(message); - } - ws.close(closeCode, reason); } diff --git a/docker/coolify-terminal/terminal-utils.test.js b/docker/coolify-terminal/terminal-utils.test.js index d0452e1f72..9f12e96b68 100644 --- a/docker/coolify-terminal/terminal-utils.test.js +++ b/docker/coolify-terminal/terminal-utils.test.js @@ -6,6 +6,7 @@ import { authenticateTerminalUpgrade, createTerminalUpgradeHandler, extractSshArgs, + getTerminalAuthHeaders, extractTargetHost, getTerminalProcessEnv, getTerminalSessionTimeout, @@ -308,18 +309,29 @@ function createFakeWebSocketServer({ handles = true } = {}) { const flushPromises = () => new Promise((resolve) => setImmediate(resolve)); -test('authenticateTerminalUpgrade rejects missing session cookies with the auth close code without calling Coolify', async () => { +test('getTerminalAuthHeaders forwards the whole cookie header so Laravel reads its own session cookie name', () => { + // APP_NAME="Coolify - Prod" gives the Laravel cookie "coolify_prod_session"; + // the terminal server must not guess that name (issue #10549). + const cookieHeader = 'theme=dark; coolify_prod_session=session-value; XSRF-TOKEN=xsrf-value'; + + assert.deepEqual(getTerminalAuthHeaders(cookieHeader, 'xsrf-value'), { + 'Cookie': cookieHeader, + 'X-XSRF-TOKEN': 'xsrf-value', + }); +}); + +test('authenticateTerminalUpgrade rejects missing cookies with the auth close code without calling Coolify', async () => { let requests = 0; const postToCoolify = async () => { requests++; return { status: 200 }; }; - for (const session of [ - { sessionCookieName: 'coolify_session', laravelSession: undefined, xsrfToken: 'xsrf' }, - { sessionCookieName: 'coolify_session', laravelSession: 'session', xsrfToken: undefined }, + for (const authHeaders of [ + getTerminalAuthHeaders('', 'xsrf'), + getTerminalAuthHeaders('coolify_session=session', undefined), ]) { - assert.deepEqual(await authenticateTerminalUpgrade(session, postToCoolify), { + assert.deepEqual(await authenticateTerminalUpgrade(authHeaders, postToCoolify), { authenticated: false, closeCode: TERMINAL_CLOSE_CODES.AUTH_REJECTED, reason: 'Unauthorized: Missing required tokens', @@ -334,7 +346,7 @@ test('authenticateTerminalUpgrade rejects missing session cookies with the auth test('authenticateTerminalUpgrade checks the Laravel session exactly like the previous verifyClient', async () => { const requests = []; const result = await authenticateTerminalUpgrade( - { sessionCookieName: 'coolify_session', laravelSession: 'session-value', xsrfToken: 'xsrf-value' }, + getTerminalAuthHeaders('coolify_session=session-value; XSRF-TOKEN=xsrf-value', 'xsrf-value'), async (path, headers) => { requests.push([path, headers]); return { status: 200 }; @@ -344,14 +356,14 @@ test('authenticateTerminalUpgrade checks the Laravel session exactly like the pr assert.deepEqual(result, { authenticated: true }); assert.deepEqual(requests, [[ '/terminal/auth', - { 'Cookie': 'coolify_session=session-value', 'X-XSRF-TOKEN': 'xsrf-value' }, + { 'Cookie': 'coolify_session=session-value; XSRF-TOKEN=xsrf-value', 'X-XSRF-TOKEN': 'xsrf-value' }, ]]); }); test('authenticateTerminalUpgrade rejects every non-200 Coolify response', async () => { for (const status of [201, 204, 302, 401, 403, 419, 500]) { const result = await authenticateTerminalUpgrade( - { sessionCookieName: 'coolify_session', laravelSession: 'session', xsrfToken: 'xsrf' }, + getTerminalAuthHeaders('coolify_session=session; XSRF-TOKEN=xsrf', 'xsrf'), async () => ({ status }), ); @@ -363,7 +375,7 @@ test('authenticateTerminalUpgrade rejects every non-200 Coolify response', async test('authenticateTerminalUpgrade does not authenticate when Coolify is unreachable', async () => { const result = await authenticateTerminalUpgrade( - { sessionCookieName: 'coolify_session', laravelSession: 'session', xsrfToken: 'xsrf' }, + getTerminalAuthHeaders('coolify_session=session; XSRF-TOKEN=xsrf', 'xsrf'), async () => { throw new Error('connect ECONNREFUSED'); }, @@ -374,18 +386,13 @@ test('authenticateTerminalUpgrade does not authenticate when Coolify is unreacha assert.equal(result.error.message, 'connect ECONNREFUSED'); }); -test('rejectTerminalSocket sends the legacy message before closing with a readable code', () => { +test('rejectTerminalSocket closes with a readable code and sends no text message', () => { const ws = createFakeWebSocket(); - rejectTerminalSocket(ws, TERMINAL_CLOSE_CODES.TOKEN_REJECTED, 'Unauthorized: Terminal token was rejected', { - message: 'Unauthorized: Terminal token was rejected', - }); + rejectTerminalSocket(ws, TERMINAL_CLOSE_CODES.TOKEN_REJECTED, 'Unauthorized: Terminal token was rejected'); ws.peerClosed(); - assert.deepEqual(ws.events, [ - ['send', 'Unauthorized: Terminal token was rejected'], - ['close', 4403, 'Unauthorized: Terminal token was rejected'], - ]); + assert.deepEqual(ws.events, [['close', 4403, 'Unauthorized: Terminal token was rejected']]); }); test('rejectTerminalSocket terminates peers that ignore the close handshake', async () => { diff --git a/resources/js/terminal-connection.js b/resources/js/terminal-connection.js index f2f463f94a..9bee050641 100644 --- a/resources/js/terminal-connection.js +++ b/resources/js/terminal-connection.js @@ -20,35 +20,17 @@ export const TERMINAL_CONNECTION_ERRORS = Object.freeze({ timeout: 'Timed out while connecting to the terminal. Reload the page and try again.', }); -const AUTH_REJECTION_MESSAGES = new Set([ - 'Unauthorized: Missing required tokens', - 'Unauthorized: Invalid terminal token', - 'Unauthorized: Terminal token was rejected', -]); - export function isTerminalAuthRejectionCode(code) { return code === TERMINAL_CLOSE_CODES.AUTH_REJECTED || code === TERMINAL_CLOSE_CODES.TOKEN_REJECTED; } /** - * Classifies a plain-text message from the terminal server. - * - * @returns {'auth-rejected'|'startup-rejected'|null} + * Whether a plain-text message from the terminal server rejects a session start, + * for example a target host that is not authorized or an invalid SSH command. + * Authentication rejections arrive as close codes instead. */ -export function classifyTerminalServerMessage(data) { - if (typeof data !== 'string') { - return null; - } - - if (AUTH_REJECTION_MESSAGES.has(data)) { - return 'auth-rejected'; - } - - if (data.startsWith('Unauthorized:') || data.startsWith('Invalid SSH command:')) { - return 'startup-rejected'; - } - - return null; +export function isTerminalStartupRejection(data) { + return typeof data === 'string' && (data.startsWith('Unauthorized:') || data.startsWith('Invalid SSH command:')); } /** diff --git a/resources/js/terminal-connection.test.js b/resources/js/terminal-connection.test.js index 2f0827283f..e6c7e4fafb 100644 --- a/resources/js/terminal-connection.test.js +++ b/resources/js/terminal-connection.test.js @@ -5,8 +5,8 @@ import { TERMINAL_CONNECT_TIMEOUT_MS, TERMINAL_CONNECTION_ERRORS, TERMINAL_SESSION_START_TIMEOUT_MS, - classifyTerminalServerMessage, isTerminalAuthRejectionCode, + isTerminalStartupRejection, resolveTerminalCloseOutcome, } from './terminal-connection.js'; @@ -70,15 +70,12 @@ test('an idle close reconnects silently until the retry limit, then reports an e }); }); -test('server text messages are classified for backward compatible rejection handling', () => { - assert.equal(classifyTerminalServerMessage('Unauthorized: Invalid terminal token'), 'auth-rejected'); - assert.equal(classifyTerminalServerMessage('Unauthorized: Terminal token was rejected'), 'auth-rejected'); - assert.equal(classifyTerminalServerMessage('Unauthorized: Missing required tokens'), 'auth-rejected'); - assert.equal(classifyTerminalServerMessage('Unauthorized: Target host 10.0.0.5 not in authorized list'), 'startup-rejected'); - assert.equal(classifyTerminalServerMessage('Invalid SSH command: No target host found'), 'startup-rejected'); - assert.equal(classifyTerminalServerMessage('pty-ready'), null); - assert.equal(classifyTerminalServerMessage('root@host:~# '), null); - assert.equal(classifyTerminalServerMessage(new Blob([])), null); +test('session start rejections from the terminal server are detected', () => { + assert.equal(isTerminalStartupRejection('Unauthorized: Target host 10.0.0.5 not in authorized list'), true); + assert.equal(isTerminalStartupRejection('Invalid SSH command: No target host found'), true); + assert.equal(isTerminalStartupRejection('pty-ready'), false); + assert.equal(isTerminalStartupRejection('root@host:~# '), false); + assert.equal(isTerminalStartupRejection(new Blob([])), false); }); test('connection timeouts stay within a bounded window', () => { diff --git a/resources/js/terminal.js b/resources/js/terminal.js index 46495425ef..2f125e92b5 100644 --- a/resources/js/terminal.js +++ b/resources/js/terminal.js @@ -9,7 +9,7 @@ import { import { TERMINAL_CONNECT_TIMEOUT_MS, TERMINAL_CONNECTION_ERRORS, - classifyTerminalServerMessage, + isTerminalStartupRejection, resolveTerminalCloseOutcome, terminalSessionStartMethods, } from './terminal-connection.js'; @@ -1107,18 +1107,12 @@ export function initializeTerminalComponent() { // Notify parent component that terminal disconnected this.$wire.dispatch('terminalDisconnected'); - } else if (classifyTerminalServerMessage(event.data) !== null) { + } else if (isTerminalStartupRejection(event.data)) { logTerminal('error', '[Terminal] Backend rejected terminal startup:', event.data); this.pendingCommand = null; this.terminalActive = false; this.resetTerminalSessionCountdown(); - // Newer servers follow an auth rejection with a 4401/4403 close. - this.failTerminalConnection( - classifyTerminalServerMessage(event.data) === 'auth-rejected' - ? TERMINAL_CONNECTION_ERRORS.authRejected - : event.data, - { override: true }, - ); + this.failTerminalConnection(event.data, { override: true }); } else { try { this.pendingWrites++;