diff --git a/app/socket_events.py b/app/socket_events.py index 6aa32d7c..0536ef0e 100644 --- a/app/socket_events.py +++ b/app/socket_events.py @@ -1547,7 +1547,7 @@ def request_auth_banner_decision(banner, context): if ((existing.socks_proxy_id or existing.via_socks or saved_socks_proxy_id) and (not existing.socks_proxy_id or existing.socks_proxy_id != saved_socks_proxy_id)): - emit_error('Saved SOCKS5 proxy route no longer matches this session') + emit_error('This persistent session uses a different SOCKS5 route. Close it and connect again.') return reconnect_tmux_name = raw_name @@ -2533,10 +2533,9 @@ def handle_delete_jump_host(data, current_user=None): @socketio.on('list_socks_proxies') @socket_login_required def handle_list_socks_proxies(current_user=None): + # Reads stay outside the mutation budget, as for jump hosts: a throttled list + # would show every routed host as having a missing proxy. try: - limited = _connection_mutation_rate_limit(current_user) - if limited: - return limited emit('socks_proxies_list', { 'socks_proxies': socks_proxy_manager.load_socks_proxies(current_user.id), }) @@ -2586,6 +2585,21 @@ def handle_delete_socks_proxy(data, current_user=None): proxy_id = data.get('socks_proxy_id') if not isinstance(proxy_id, str) or not proxy_id or len(proxy_id) > 64: return _command_set_error('SOCKS proxy ID required') + # Reattaching a persistent tmux session requires its original proxy, so + # deleting that proxy would strand the session. + persistent = SSHSession.query.filter_by( + user_id=current_user.id, is_persistent=True, socks_proxy_id=proxy_id, + ).order_by(SSHSession.id).all() + if persistent: + labels = [ + session.display_name or f'{session.username}@{session.host}' + for session in persistent[:20] + ] + noun = 'session' if len(persistent) == 1 else 'sessions' + return _command_set_error( + f'SOCKS proxy is used by {len(persistent)} persistent {noun}', + labels, + ) success, error, usages = socks_proxy_manager.delete_socks_proxy(current_user.id, proxy_id) if not success: return _command_set_error(error, usages) diff --git a/app/socks5_client.py b/app/socks5_client.py index 7ff58010..80934beb 100644 --- a/app/socks5_client.py +++ b/app/socks5_client.py @@ -174,6 +174,10 @@ def open_socks5_tunnel(sock, destination, *, username=None, password=None, version, reply, reserved, address_type = _recv_exact(sock, 4, deadline, cancelled) if version != 5 or reserved != 0: raise Socks5Error('socks_protocol_error') + # The socket is closed on failure, so a failed reply needs no bound + # address; proxies that truncate it still report the real reason. + if reply != 0: + raise Socks5Error(_REPLY_CODES.get(reply, 'socks_protocol_error')) if address_type == 1: address_length = 4 elif address_type == 4: @@ -186,8 +190,6 @@ def open_socks5_tunnel(sock, destination, *, username=None, password=None, raise Socks5Error('socks_protocol_error') _recv_exact(sock, address_length, deadline, cancelled) _recv_exact(sock, 2, deadline, cancelled) - if reply != 0: - raise Socks5Error(_REPLY_CODES.get(reply, 'socks_protocol_error')) _remaining(deadline, cancelled) succeeded = True except Socks5Error as error: diff --git a/app/ssh_manager.py b/app/ssh_manager.py index 38741155..1b791777 100644 --- a/app/ssh_manager.py +++ b/app/ssh_manager.py @@ -428,11 +428,14 @@ def connection_cancelled(): remote_dns_allowlist=config.SOCKS_REMOTE_DNS_ALLOWLIST, ) host, port = destination.hostname, destination.port - proxy_target = resolve_allowed_target( - socks_proxy.host, socks_proxy.port, - allow_internal=not config.BLOCK_INTERNAL_SSH, - allowed_internal_networks=config.SSH_ALLOWED_INTERNAL_NETWORKS, - ) + try: + proxy_target = resolve_allowed_target( + socks_proxy.host, socks_proxy.port, + allow_internal=not config.BLOCK_INTERNAL_SSH, + allowed_internal_networks=config.SSH_ALLOWED_INTERNAL_NETWORKS, + ) + except ValueError as error: + raise ValueError(f'SOCKS5 proxy: {error}') from error audit_internal_allowlist_use(proxy_target, 'socks_proxy') try: validated_socket = open_validated_socket(proxy_target, config.SSH_CONNECT_TIMEOUT) diff --git a/docs/wiki/Data-Storage-and-Persistence.md b/docs/wiki/Data-Storage-and-Persistence.md index fdd252a8..a01b1bb6 100644 --- a/docs/wiki/Data-Storage-and-Persistence.md +++ b/docs/wiki/Data-Storage-and-Persistence.md @@ -108,6 +108,14 @@ The additive migration leaves existing rows without a proxy and sets JSON migrations from unversioned data through versions 1 and 2 only add or advance the schema marker. +Releases without SOCKS5 support keep an unknown `socks_proxy_id` in profiles but +ignore it, so a downgraded instance connects those hosts directly. Saved +passwords stay unusable there because their route binding no longer matches, but +SSH keys and passwords entered at connect time would reach the target without +the proxy. Before downgrading, remove the SOCKS5 route from every host in the +updated application, or restore the complete pre-upgrade backup with its +matching secret into a separate directory. + ### Workspace upgrade and rollback The workspace update adds a nullable `profile_id` column to saved SSH sessions. diff --git a/docs/wiki/Profiles-Jump-Hosts-and-Commands.md b/docs/wiki/Profiles-Jump-Hosts-and-Commands.md index de4481ba..f829f404 100644 --- a/docs/wiki/Profiles-Jump-Hosts-and-Commands.md +++ b/docs/wiki/Profiles-Jump-Hosts-and-Commands.md @@ -157,7 +157,9 @@ review, and a failed proxy never causes a direct connection attempt. **Edit** keeps the proxy's ID; dependent hosts use its current configuration on their next connection. Existing sessions are not restarted. **Delete** requires confirmation and is blocked while saved hosts reference the proxy; the error -lists up to 20 affected host names and the total count. If a save times out, +lists up to 20 affected host names and the total count. It is also blocked while +a persistent tmux session was opened through the proxy, because that session can +only be resumed over its original route; close the session first. If a save times out, check the saved list before retrying because the write may already have succeeded. SOCKS5 routes are unsupported in standalone **Quick SFTP** and Automation runs diff --git a/static/js/app.js b/static/js/app.js index 8fa4f4a5..479c255b 100644 --- a/static/js/app.js +++ b/static/js/app.js @@ -279,6 +279,10 @@ }; function openConnectionAssetManager(asset) { + // Resource views open behind the connection dialog, so leave it first. + if (document.getElementById('connectionModal')?.classList.contains('show')) { + dismissConnectionModal(); + } if (asset === 'passwords') { window.primaryWorkspaceController?.open('hosts', document.getElementById('credentialManagementView')); window.NativeCredentials?.mount(); @@ -303,7 +307,7 @@ ProfileManager.loadKeys(); if (asset === 'jump-hosts') { window.JumpHostManager?.load(); - window.SocksProxyManager?.load(); + window.SocksProxyManager?.load(); window.JumpHostManager?.renderList(); } const view = document.getElementById(modalId); @@ -432,6 +436,11 @@ const unresolved = !proxy || Boolean(session.jumpHostId || session.viaJump) || session.reconnectRouteKnown === false; if (applyToForm) { selectedConnectionProfileState = {requiredSocksProxyId: session.socksProxyId || '', missingSocksProxy: unresolved}; + const jumpSelect = document.getElementById('jumpHostSelect'); + if (jumpSelect) jumpSelect.value = ''; + const jumpPassword = document.getElementById('jumpHostPasswordInput'); + if (jumpPassword) jumpPassword.value = ''; + window.JumpHostManager?.updatePasswordVisibility(); manager?.selectReference(document.getElementById('socksProxySelect'), session.socksProxyId); const password = document.getElementById('socksProxyPasswordInput'); if (password) password.value = ''; @@ -450,6 +459,10 @@ || session.reconnectRouteKnown === false); if (applyToForm) { selectedConnectionProfileState = {missingJumpHost: unresolved}; + const socksSelect = document.getElementById('socksProxySelect'); + if (socksSelect) socksSelect.value = ''; + const socksPassword = document.getElementById('socksProxyPasswordInput'); + if (socksPassword) socksPassword.value = ''; const select = document.getElementById('jumpHostSelect'); if (select) select.value = jump ? requiredId : ''; // Setting the value in code fires no change event; apply the jump host's @@ -460,6 +473,8 @@ window.CredentialPicker?.restore?.('jumpHostPasswordInput'); } manager?.updatePasswordVisibility(); + window.SocksProxyManager?.updatePasswordVisibility(); + window.SocksProxyManager?.updateRouteControls(); window.setConnectionAdvancedExpanded(Boolean(jump || unresolved)); refreshConnectionProfileJumpResolution(); if (jump?.auth_type === 'password') { diff --git a/static/js/i18n.js b/static/js/i18n.js index d423cdec..a03da66e 100644 --- a/static/js/i18n.js +++ b/static/js/i18n.js @@ -25,8 +25,8 @@ const translations = { "socksproxies.deleted": "SOCKS5 proxy deleted", "socksproxies.confirmDelete": "Delete this SOCKS5 proxy?", "socksproxies.inUse": "This SOCKS5 proxy is used by {count} host(s)", + "socksproxies.inUseSessions": "This SOCKS5 proxy is still used by {count} persistent session(s). Close them before deleting the proxy", "socksproxies.saveUnknownHint": "Save result unknown. Check the saved list before trying again. Your draft is preserved.", - "socksproxies.saveFailed": "Could not save the SOCKS5 proxy.", "socksproxies.loading": "Loading SOCKS5 proxies…", "connection.route": "Route", "connection.routeClearJump": "A host uses one route: clear the jump host to choose a SOCKS5 proxy.", @@ -35,7 +35,7 @@ const translations = { "connection.routeConflict": "Choose one compatible route.", "connection.socksProxy": "SOCKS5 Proxy", "connection.socksProxyNone": "None", - "connection.socksProxyManage": "Manage SOCKS5 proxies in the connection resources.", + "connection.socksProxyManage": "Manage SOCKS5 proxies", "connection.socksProxyPassword": "SOCKS5 Proxy Password", "connection.socksProxyPasswordHint": "This proxy uses password login. Enter its password; it is not stored unless you save it.", "connection.socksProxyPasswordRequired": "SOCKS5 proxy password is required", @@ -2521,8 +2521,8 @@ const translations = { "socksproxies.deleted": "Đã xóa proxy SOCKS5", "socksproxies.confirmDelete": "Xóa proxy SOCKS5 này?", "socksproxies.inUse": "Proxy SOCKS5 này đang được {count} máy chủ sử dụng", + "socksproxies.inUseSessions": "Proxy SOCKS5 này vẫn đang được {count} phiên lâu dài sử dụng. Hãy đóng các phiên đó trước khi xóa proxy", "socksproxies.saveUnknownHint": "Chưa rõ kết quả lưu. Kiểm tra danh sách đã lưu trước khi thử lại. Bản nháp vẫn được giữ.", - "socksproxies.saveFailed": "Không thể lưu proxy SOCKS5.", "socksproxies.loading": "Đang tải proxy SOCKS5…", "connection.route": "Tuyến kết nối", "connection.routeClearJump": "Máy chủ chỉ dùng một tuyến: bỏ máy chủ trung chuyển để chọn proxy SOCKS5.", @@ -2531,7 +2531,7 @@ const translations = { "connection.routeConflict": "Chọn một tuyến kết nối tương thích.", "connection.socksProxy": "Proxy SOCKS5", "connection.socksProxyNone": "Không có", - "connection.socksProxyManage": "Quản lý proxy SOCKS5 trong tài nguyên kết nối.", + "connection.socksProxyManage": "Quản lý proxy SOCKS5", "connection.socksProxyPassword": "Mật khẩu proxy SOCKS5", "connection.socksProxyPasswordHint": "Proxy này dùng mật khẩu để đăng nhập. Hãy nhập mật khẩu; mật khẩu chỉ được lưu khi bạn yêu cầu.", "connection.socksProxyPasswordRequired": "Cần nhập mật khẩu proxy SOCKS5", @@ -5011,8 +5011,8 @@ const translations = { "socksproxies.deleted": "SOCKS5-Proxy gelöscht", "socksproxies.confirmDelete": "Diesen SOCKS5-Proxy löschen?", "socksproxies.inUse": "Dieser SOCKS5-Proxy wird von {count} Host(s) verwendet", + "socksproxies.inUseSessions": "Dieser SOCKS5-Proxy wird noch von {count} persistenten Session(s) verwendet. Schließe sie, bevor du den Proxy löschst", "socksproxies.saveUnknownHint": "Speicherergebnis unbekannt. Prüfe vor einem weiteren Versuch die gespeicherte Liste. Dein Entwurf bleibt erhalten.", - "socksproxies.saveFailed": "SOCKS5-Proxy konnte nicht gespeichert werden.", "socksproxies.loading": "SOCKS5-Proxys werden geladen…", "connection.route": "Route", "connection.routeClearJump": "Ein Host verwendet eine Route: Entferne den Jump Host, um einen SOCKS5-Proxy zu wählen.", @@ -5021,7 +5021,7 @@ const translations = { "connection.routeConflict": "Wähle eine kompatible Route.", "connection.socksProxy": "SOCKS5-Proxy", "connection.socksProxyNone": "Keiner", - "connection.socksProxyManage": "SOCKS5-Proxys in den Verbindungsressourcen verwalten.", + "connection.socksProxyManage": "SOCKS5-Proxys verwalten", "connection.socksProxyPassword": "SOCKS5-Proxy-Passwort", "connection.socksProxyPasswordHint": "Dieser Proxy verwendet eine Passwort-Anmeldung. Gib sein Passwort ein; es wird nur auf ausdrücklichen Wunsch gespeichert.", "connection.socksProxyPasswordRequired": "SOCKS5-Proxy-Passwort erforderlich", @@ -7505,8 +7505,8 @@ const translations = { "socksproxies.deleted": "Proxy SOCKS5 supprimé", "socksproxies.confirmDelete": "Supprimer ce proxy SOCKS5 ?", "socksproxies.inUse": "Ce proxy SOCKS5 est utilisé par {count} hôte(s)", + "socksproxies.inUseSessions": "Ce proxy SOCKS5 est encore utilisé par {count} session(s) persistante(s). Fermez-les avant de supprimer le proxy", "socksproxies.saveUnknownHint": "Résultat inconnu. Vérifiez la liste avant de réessayer. Votre brouillon est conservé.", - "socksproxies.saveFailed": "Impossible d’enregistrer le proxy SOCKS5.", "socksproxies.loading": "Chargement des proxys SOCKS5…", "connection.route": "Route", "connection.routeClearJump": "Un hôte utilise une seule route : retirez l’hôte de rebond pour choisir un proxy SOCKS5.", @@ -7515,7 +7515,7 @@ const translations = { "connection.routeConflict": "Choisissez une seule route compatible.", "connection.socksProxy": "Proxy SOCKS5", "connection.socksProxyNone": "Aucun", - "connection.socksProxyManage": "Gérer les proxys SOCKS5 dans les ressources de connexion.", + "connection.socksProxyManage": "Gérer les proxys SOCKS5", "connection.socksProxyPassword": "Mot de passe du proxy SOCKS5", "connection.socksProxyPasswordHint": "Ce proxy utilise un mot de passe. Saisissez-le ; il n’est conservé que si vous l’enregistrez.", "connection.socksProxyPasswordRequired": "Le mot de passe du proxy SOCKS5 est requis", @@ -9995,8 +9995,8 @@ const translations = { "socksproxies.deleted": "Proxy SOCKS5 eliminado", "socksproxies.confirmDelete": "¿Eliminar este proxy SOCKS5?", "socksproxies.inUse": "Este proxy SOCKS5 lo usan {count} hosts", + "socksproxies.inUseSessions": "Este proxy SOCKS5 todavía lo usan {count} sesiones persistentes. Ciérralas antes de eliminar el proxy", "socksproxies.saveUnknownHint": "Resultado desconocido. Revisa la lista antes de reintentarlo. Se conserva tu borrador.", - "socksproxies.saveFailed": "No se pudo guardar el proxy SOCKS5.", "socksproxies.loading": "Cargando proxies SOCKS5…", "connection.route": "Ruta", "connection.routeClearJump": "Un host usa una sola ruta: quita el host de salto para elegir un proxy SOCKS5.", @@ -10005,7 +10005,7 @@ const translations = { "connection.routeConflict": "Elige una sola ruta compatible.", "connection.socksProxy": "Proxy SOCKS5", "connection.socksProxyNone": "Ninguno", - "connection.socksProxyManage": "Administra los proxies SOCKS5 en los recursos de conexión.", + "connection.socksProxyManage": "Administrar proxies SOCKS5", "connection.socksProxyPassword": "Contraseña del proxy SOCKS5", "connection.socksProxyPasswordHint": "Este proxy usa contraseña. Introdúcela; solo se almacena si la guardas.", "connection.socksProxyPasswordRequired": "Se requiere la contraseña del proxy SOCKS5", @@ -12485,8 +12485,8 @@ const translations = { "socksproxies.deleted": "SOCKS5 代理已删除", "socksproxies.confirmDelete": "删除此 SOCKS5 代理？", "socksproxies.inUse": "此 SOCKS5 代理正被 {count} 台主机使用", + "socksproxies.inUseSessions": "此 SOCKS5 代理仍被 {count} 个持久会话使用。请先关闭这些会话再删除代理", "socksproxies.saveUnknownHint": "保存结果未知。再次尝试前请检查已保存列表。草稿已保留。", - "socksproxies.saveFailed": "无法保存 SOCKS5 代理。", "socksproxies.loading": "正在加载 SOCKS5 代理…", "connection.route": "连接路由", "connection.routeClearJump": "一台主机只能使用一条路由：清除跳板机后才能选择 SOCKS5 代理。", @@ -12495,7 +12495,7 @@ const translations = { "connection.routeConflict": "请选择一条兼容的连接路由。", "connection.socksProxy": "SOCKS5 代理", "connection.socksProxyNone": "无", - "connection.socksProxyManage": "在连接资源中管理 SOCKS5 代理。", + "connection.socksProxyManage": "管理 SOCKS5 代理", "connection.socksProxyPassword": "SOCKS5 代理密码", "connection.socksProxyPasswordHint": "此代理使用密码登录。请输入密码；只有明确保存时才会存储。", "connection.socksProxyPasswordRequired": "请输入 SOCKS5 代理密码", diff --git a/static/js/socks-proxy-manager.js b/static/js/socks-proxy-manager.js index 93281d2d..b04c8724 100644 --- a/static/js/socks-proxy-manager.js +++ b/static/js/socks-proxy-manager.js @@ -278,8 +278,6 @@ window.SocksProxyManager = { this.unknownOutcome = true; window.showNotification?.(this.t('common.saveUnknownTitle', 'Save outcome unknown'), 'error'); this.load(); - } else if (!result?.success) { - window.showNotification?.(result?.error || this.t('socksproxies.saveFailed', 'Could not save the SOCKS5 proxy.'), 'error'); } else if (result?.success) { if (!current()) return; this.upsertSocksProxy(result.socks_proxy); @@ -298,9 +296,11 @@ window.SocksProxyManager = { }, handleUsageError(data) { - const count = /^SOCKS proxy is used by (\d+)/.exec(data?.error || '')?.[1]; - if (!count || !Array.isArray(data.usages)) return false; - const message = this.t('socksproxies.inUse', 'This SOCKS5 proxy is used by {count} host(s)').replace('{count}', count); + const match = /^SOCKS proxy is used by (\d+)( persistent)?/.exec(data?.error || ''); + if (!match || !Array.isArray(data.usages)) return false; + const message = (match[2] + ? this.t('socksproxies.inUseSessions', 'This SOCKS5 proxy is still used by {count} persistent session(s). Close them before deleting the proxy') + : this.t('socksproxies.inUse', 'This SOCKS5 proxy is used by {count} host(s)')).replace('{count}', match[1]); window.showNotification?.(message + ': ' + data.usages.slice(0, 20).join(', '), 'error'); return true; }, diff --git a/templates/index.html b/templates/index.html index 5f115a23..65c9f508 100644 --- a/templates/index.html +++ b/templates/index.html @@ -771,7 +771,7 @@

Connection Detail
- +
-
+
diff --git a/tests/e2e/socks-proxy-routes.spec.js b/tests/e2e/socks-proxy-routes.spec.js index 583b4b5c..d1c650dc 100644 --- a/tests/e2e/socks-proxy-routes.spec.js +++ b/tests/e2e/socks-proxy-routes.spec.js @@ -182,6 +182,38 @@ test('deletion of a referenced SOCKS proxy reports its hosts', async ({page}) => await expect(page.locator('.notification').filter({hasText:/SOCKS.*used by/})).toHaveCount(1); }); +test('managing proxies from the connection dialog leaves the dialog', async ({page}) => { + await login(page); + await page.locator('#workspaceRecentBtn').click(); + await expect(page.locator('#connectionModal')).toHaveClass(/\bshow\b/); + await page.locator('#connectionAdvancedSettings').evaluate(element => { element.open = true; }); + await page.locator('#connectionModal [data-connection-asset="socks-proxies"]').click(); + await expect(page.locator('#connectionModal')).not.toHaveClass(/\bshow\b/); + await expect(page.locator('#socksProxyManagementModal')).toBeVisible(); + await expect(page.locator('#socksProxiesList .key-item').first()).toBeVisible(); +}); + +test('programmatic jump and SOCKS routes replace each other in the connection form', async ({page}) => { + await login(page); + await page.locator('#workspaceRecentBtn').click(); + await page.locator('#connectionAdvancedSettings').evaluate(element => { element.open = true; }); + await expect.poll(() => page.evaluate(() => JumpHostManager.loaded && SocksProxyManager.loaded)).toBe(true); + const ids = await page.evaluate(() => ({ + jump: JumpHostManager.jumpHosts[0].id, + socks: SocksProxyManager.socksProxies.find(proxy => proxy.auth_type === 'none').id, + })); + await page.locator('#socksProxySelect').selectOption(ids.socks); + await page.evaluate(id => window.prepareSessionReconnectJump({jumpHostId: id, reconnectRouteKnown: true}, {applyToForm: true}), ids.jump); + await expect(page.locator('#jumpHostSelect')).toHaveValue(ids.jump); + await expect(page.locator('#socksProxySelect')).toHaveValue(''); + await expect(page.locator('#socksProxySelect')).toBeDisabled(); + await expect(page.locator('#connectionRouteHint')).toBeVisible(); + await page.evaluate(id => window.prepareSessionReconnectJump({socksProxyId: id, viaSocks: true, reconnectRouteKnown: true}, {applyToForm: true}), ids.socks); + await expect(page.locator('#socksProxySelect')).toHaveValue(ids.socks); + await expect(page.locator('#jumpHostSelect')).toHaveValue(''); + await expect(page.locator('#jumpHostSelect')).toBeDisabled(); +}); + test('SOCKS overview and route map identify the proxy and missing references', async ({page}) => { const errors=[]; page.on('pageerror',error=>errors.push(error.message)); diff --git a/tests/js/socks-proxy-manager.test.js b/tests/js/socks-proxy-manager.test.js new file mode 100644 index 00000000..b00493e0 --- /dev/null +++ b/tests/js/socks-proxy-manager.test.js @@ -0,0 +1,45 @@ +const test = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const vm = require('node:vm'); + +function setup(response) { + const notifications = []; + const sent = []; + const element = {reportValidity: () => true, dataset: {}, classList: {contains: () => true}, hidden: false}; + const window = { + addEventListener() {}, + showNotification: (message, type) => notifications.push({message, type}), + socket: { + connected: true, + timeout: () => ({emit: (event, payload, callback) => { sent.push({event, payload}); callback(null, response); }}), + }, + }; + const context = {window, document: {getElementById: () => element}}; + vm.runInNewContext(fs.readFileSync(path.join(__dirname, '../../static/js/socks-proxy-manager.js'), 'utf8'), context); + const manager = window.SocksProxyManager; + manager.renderEditorState = manager.renderList = () => {}; + manager.draft = () => 'draft'; + return {manager, notifications, sent}; +} + +test('a rejected save leaves the server error to the global error handler', async () => { + const {manager, notifications, sent} = setup({success: false, error: 'Invalid host format', code: 'validation_error'}); + await manager.save('Proxy', 'bad host!', '1080', '', 'none', true); + assert.equal(sent.length, 1); + assert.equal(sent[0].event, 'save_socks_proxy'); + assert.deepEqual(notifications, []); + assert.equal(manager.pending, false); +}); + +test('usage errors name hosts and persistent sessions separately', () => { + const {manager, notifications} = setup(); + assert.equal(manager.handleUsageError({error: 'SOCKS proxy is used by 2 profiles', usages: ['A', 'B']}), true); + assert.equal(manager.handleUsageError({error: 'SOCKS proxy is used by 1 persistent session', usages: ['Build box']}), true); + assert.equal(manager.handleUsageError({error: 'Invalid host format'}), false); + assert.deepEqual(notifications.map(item => item.message), [ + 'This SOCKS5 proxy is used by 2 host(s): A, B', + 'This SOCKS5 proxy is still used by 1 persistent session(s). Close them before deleting the proxy: Build box', + ]); +}); diff --git a/tests/test_socks5_client.py b/tests/test_socks5_client.py index 4816f0c0..324ace44 100644 --- a/tests/test_socks5_client.py +++ b/tests/test_socks5_client.py @@ -204,6 +204,24 @@ def test_reply_errors_have_stable_safe_codes_and_close_socket(reply, code, messa assert str(error) == message +@pytest.mark.parametrize('reply_header,code', [ + (b'\x05\x02\x00\x01', 'socks_not_allowed'), + (b'\x05\x05\x00\x00', 'socks_connection_refused'), + (b'\x05\x04\x00\x03', 'socks_host_unreachable'), + (b'\x05\x09\x00\x01', 'socks_protocol_error'), + (b'\x05\x02\x01\x01', 'socks_protocol_error'), +]) +def test_failure_reply_reports_reason_without_bound_address(reply_header, code): + def script(connection): + _exchange([ + (b'\x05\x01\x00', b'\x05\x00'), + (_IPV4_REQUEST, reply_header), + ])(connection) + _expect_closed(connection) + + _assert_failure(script, code) + + @pytest.mark.parametrize('credentials,greeting,response,code', [ ({}, b'\x05\x01\x00', b'\x05\xff', 'socks_auth_required'), ({'username': 'user', 'password': 'secret'}, b'\x05\x01\x02', b'\x05\xff', 'socks_auth_rejected'), diff --git a/tests/test_socks_connection_socket.py b/tests/test_socks_connection_socket.py index 059bcb0c..f9ed3e98 100644 --- a/tests/test_socks_connection_socket.py +++ b/tests/test_socks_connection_socket.py @@ -174,7 +174,7 @@ def test_persistent_tmux_reconnect_cannot_drop_or_replace_socks_route(app, monke _, emitted = call(app, monkeypatch, events.handle_ssh_connect, sid, connect_payload(use_tmux=True, reconnect_tmux_name='saved_tmux', **extra)) assert not jobs - assert emitted[0][1]['error'] == 'Saved SOCKS5 proxy route no longer matches this session' + assert emitted[0][1]['error'] == 'This persistent session uses a different SOCKS5 route. Close it and connect again.' def test_combined_credential_check_bypasses_cached_poll_result(): diff --git a/tests/test_socks_proxy_socket.py b/tests/test_socks_proxy_socket.py index cf1e5588..602b9947 100644 --- a/tests/test_socks_proxy_socket.py +++ b/tests/test_socks_proxy_socket.py @@ -85,6 +85,56 @@ def test_socks_crud_rejects_used_proxy_and_limits_mutations(app, monkeypatch): assert result == {'success': False, 'error': 'limited'} +def test_socks_delete_keeps_proxy_of_persistent_sessions(app, monkeypatch): + from app import socks_proxy_manager + from app.models import SSHSession, db + owner, sid = user_socket(app, 'socks_persistent') + other, _ = user_socket(app, 'socks_persistent_other') + with app.app_context(): + proxy, error = socks_proxy_manager.add_socks_proxy(owner, 'Proxy', 'proxy.example', 1080) + assert error is None + + def session_row(session_id, user_id, persistent, display_name=None): + db.session.add(SSHSession( + session_id=session_id, user_id=user_id, host='target.example', port=22, + username='ops', connected=False, is_persistent=persistent, + tmux_session_name='webssh_' + session_id.replace('-', '_'), + display_name=display_name, socks_proxy_id=proxy['id'], via_socks=True, + )) + + session_row('plain-session', owner, False) + session_row('foreign-session', other, True) + session_row('tmux-session', owner, True, display_name='Build box') + db.session.commit() + + denied, emitted = call(app, monkeypatch, events.handle_delete_socks_proxy, sid, + {'socks_proxy_id': proxy['id']}) + assert denied == { + 'success': False, + 'error': 'SOCKS proxy is used by 1 persistent session', + 'code': 'in_use', + 'usages': ['Build box'], + } + assert emitted == [('error', denied)] + with app.app_context(): + assert socks_proxy_manager.get_socks_proxy(owner, proxy['id']) is not None + SSHSession.query.filter_by(session_id='tmux-session').delete() + db.session.commit() + + deleted, _ = call(app, monkeypatch, events.handle_delete_socks_proxy, sid, + {'socks_proxy_id': proxy['id']}) + assert deleted == {'success': True, 'socks_proxy_id': proxy['id']} + + +def test_socks_list_is_not_limited_by_mutation_budget(app, monkeypatch): + _, sid = user_socket(app, 'socks_list_budget') + monkeypatch.setattr(events, '_connection_mutation_rate_limit', + lambda user: pytest.fail('listing consumed the mutation budget')) + result, emitted = call(app, monkeypatch, events.handle_list_socks_proxies, sid) + assert result is None + assert emitted == [('socks_proxies_list', {'socks_proxies': []})] + + @pytest.mark.parametrize('payload', [{}, {'port': True}, {'remote_dns': 'false'}]) def test_socks_crud_invalid_payload_is_safe(app, monkeypatch, payload): _, sid = user_socket(app, 'socks_invalid') diff --git a/tests/test_ssh_manager.py b/tests/test_ssh_manager.py index 320f8ed1..4764e852 100644 --- a/tests/test_ssh_manager.py +++ b/tests/test_ssh_manager.py @@ -295,6 +295,8 @@ def test_socks_blocked_target_or_proxy_opens_no_socket(monkeypatch, host, proxy_ session_id, error = connect_target(host=host, socks_proxy=socks_route(host=proxy_host)) assert session_id is None assert 'not allowed' in str(error) + # Users must be able to tell a blocked proxy from a blocked target. + assert str(error).startswith('SOCKS5 proxy:') is (proxy_host == '127.0.0.1') assert clients.opened_sockets == [] assert clients == []