mirror of
https://github.com/coollabsio/coolify.git
synced 2026-11-06 13:10:37 -04:00
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 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
4f9ad86e52
commit
3a4ea3b4d6
@@ -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) {
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
|
||||
@@ -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 () => {
|
||||
|
||||
@@ -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:'));
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -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', () => {
|
||||
|
||||
@@ -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++;
|
||||
|
||||
Reference in New Issue
Block a user