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++;